[AA] Correctly maintain the sign of PartiaAlias offset

[chill]

Summary

Preserve the invariant that offset reported in the case of a
PartialAlias between Loc1 and Loc2, is such that Loc1 + Offset = Loc2, where
Loc1 and Loc2 are the first and the second argument, respectively, in alias queries.

[nikic]

Nice catch, LGTM!

[dfukalov]

llvm/unittests/Analysis/AliasAnalysisTest.cpp

364–366

Thanks!
Nit: I guess you can just extend the previous test "PartialAliasOffset" with this lines (and changed "-1" to "-4").

[dfukalov]

llvm/unittests/Analysis/AliasAnalysisTest.cpp

364–366

Sorry, I was wrong here - you indeed need separate test where Loc1 is not GEP.

llvm/lib/Analysis/BasicAliasAnalysis.cpp

AliasResult BasicAAResult::aliasCheckRecursive(
     const Value *V2, LocationSize V2Size,
     AAQueryInfo &AAQI, const Value *O1, const Value *O2) {
   if (const GEPOperator *GV1 = dyn_cast<GEPOperator>(V1)) {
     AliasResult Result = aliasGEP(GV1, V1Size, V2, V2Size, O1, O2, AAQI);
     if (Result != AliasResult::MayAlias)
       return Result;
   } else if (const GEPOperator *GV2 = dyn_cast<GEPOperator>(V2)) {
     AliasResult Result = aliasGEP(GV2, V2Size, V1, V1Size, O2, O1, AAQI);
+    Result.swap();
     if (Result != AliasResult::MayAlias)
       return Result;
   }
 
   if (const PHINode *PN = dyn_cast<PHINode>(V1)) {
     AliasResult Result = aliasPHI(PN, V1Size, V2, V2Size, AAQI);
     if (Result != AliasResult::MayAlias)
       return Result;
   } else if (const PHINode *PN = dyn_cast<PHINode>(V2)) {
     AliasResult Result = aliasPHI(PN, V2Size, V1, V1Size, AAQI);
+    Result.swap();
     if (Result != AliasResult::MayAlias)
       return Result;
   }
 
   if (const SelectInst *S1 = dyn_cast<SelectInst>(V1)) {
     AliasResult Result = aliasSelect(S1, V1Size, V2, V2Size, AAQI);
     if (Result != AliasResult::MayAlias)
       return Result;
   } else if (const SelectInst *S2 = dyn_cast<SelectInst>(V2)) {
     AliasResult Result = aliasSelect(S2, V2Size, V1, V1Size, AAQI);
+    Result.swap();
     if (Result != AliasResult::MayAlias)
       return Result;
   }
 
   // If both pointers are pointing into the same object and one of them
   // accesses the entire object, then the accesses must overlap in some way.
   if (O1 == O2) {
     bool NullIsValidLocation = NullPointerIsDefined(&F);

llvm/unittests/Analysis/AliasAnalysisTest.cpp

TEST_F(AliasAnalysisTest, PartialAliasOffset) {
 
   auto &AA = getAAResults(*F);
 
   const auto AR = AA.alias(Loc1, Loc2);
   EXPECT_EQ(AR, AliasResult::PartialAlias);
   EXPECT_EQ(4, AR.getOffset());
 }
 
+// Check that swapping the order of parameters to `AA.alias()` changes offset
+// sign and that the sign is such that FirstLoc + Offset == SecondLoc.
+TEST_F(AliasAnalysisTest, PartialAliasOffsetSign) {
+  LLVMContext C;
+  SMDiagnostic Err;
+  std::unique_ptr<Module> M = parseAssemblyString(R"(
+    define void @f(i64* %p) {
+      %L1 = load i64, i64* %p
+      %p.i8 = bitcast i64* %p to i8*
+      %q = getelementptr i8,  i8* %p.i8, i32 1
+      %L2 = load i8, i8* %q
+      ret void
+    }
+  )",
+                                                  Err, C);
+
+  if (!M)
+    Err.print("PartialAliasOffsetSign", errs());
+
+  Function *F = M->getFunction("f");
+  const auto Loc1 = MemoryLocation::get(getInstructionByName(*F, "L1"));
+  const auto Loc2 = MemoryLocation::get(getInstructionByName(*F, "L2"));
+
+  auto &AA = getAAResults(*F);
+
+  auto AR = AA.alias(Loc1, Loc2);
+  EXPECT_EQ(AR, AliasResult::PartialAlias);
+  EXPECT_EQ(1, AR.getOffset());
+
+  AR = AA.alias(Loc2, Loc1);
+  EXPECT_EQ(AR, AliasResult::PartialAlias);
+  EXPECT_EQ(-1, AR.getOffset());
+}
 class AAPassInfraTest : public testing::Test {
 protected:
   LLVMContext C;
   SMDiagnostic Err;
   std::unique_ptr<Module> M;
 
 public:
   AAPassInfraTest()