[AST] Fix a bug in aliasesUnknownInst. Make sure we are comparing the unknown instructions in the alias set and the instruction interested in.

[trentxintong]

Summary

Make sure we are comparing the unknown instructions in the alias set and the instruction interested in.
I believe this is clearly a bug (missed opportunity). I can also add some test cases if desired.

[davide]

Yes, please add some tests.

[trentxintong]

Add test.

[trentxintong]

Add comments

[trentxintong]

Add 1 more assert in the test case.

[hfinkel]

LGTM

llvm/trunk/lib/Analysis/AliasSetTracker.cpp

bool AliasSet::aliasesUnknownInst(const Instruction *Inst,
 
   if (AliasAny)
     return true;
 
   if (!Inst->mayReadOrWriteMemory())
     return false;
 
   for (unsigned i = 0, e = UnknownInsts.size(); i != e; ++i) {
-    if (auto *Inst = getUnknownInst(i)) {
-      ImmutableCallSite C1(Inst), C2(Inst);
+    if (auto *UnknownInst = getUnknownInst(i)) {
+      ImmutableCallSite C1(UnknownInst), C2(Inst);
       if (!C1 || !C2 || AA.getModRefInfo(C1, C2) != MRI_NoModRef ||
           AA.getModRefInfo(C2, C1) != MRI_NoModRef)
         return true;
     }
   }
 
   for (iterator I = begin(), E = end(); I != E; ++I)
     if (AA.getModRefInfo(Inst, MemoryLocation(I.getPointer(), I.getSize(),

llvm/trunk/unittests/Analysis/AliasSetTrackerTest.cpp

+//=======- AliasSetTrackerTest.cpp - Unit test for the Alias Set Tracker  -===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Analysis/AliasAnalysis.h"
+#include "llvm/Analysis/AliasSetTracker.h"
+#include "llvm/Analysis/TypeBasedAliasAnalysis.h"
+#include "llvm/AsmParser/Parser.h"
+#include "llvm/IR/LLVMContext.h"
+#include "llvm/IR/Module.h"
+#include "llvm/Support/SourceMgr.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+
+TEST(AliasSetTracker, AliasUnknownInst) {
+  StringRef Assembly = R"(
+    @a = common global i32 0, align 4
+    @b = common global float 0.000000e+00, align 4
+
+    ; Function Attrs: nounwind ssp uwtable
+    define i32 @read_a() #0 {
+      %1 = load i32, i32* @a, align 4, !tbaa !3
+      ret i32 %1
+    }
+
+    ; Function Attrs: nounwind ssp uwtable
+    define void @write_b() #0 {
+      store float 1.000000e+01, float* @b, align 4, !tbaa !7
+      ret void
+    }
+
+    ; Function Attrs: nounwind ssp uwtable
+    define void @test() #0 {
+      %1 = call i32 @read_a(), !tbaa !3
+      call void @write_b(), !tbaa !7
+      ret void
+    }
+
+    !3 = !{!4, !4, i64 0}
+    !4 = !{!"int", !5, i64 0}
+    !5 = !{!"omnipotent char", !6, i64 0}
+    !6 = !{!"Simple C/C++ TBAA"}
+    !7 = !{!8, !8, i64 0}
+    !8 = !{!"float", !5, i64 0}
+  )";
+
+  // Parse the IR. The two calls in @test can not access aliasing elements.
+  LLVMContext Context;
+  SMDiagnostic Error;
+  auto M = parseAssemblyString(Assembly, Error, Context);
+  ASSERT_TRUE(M) << "Bad assembly?";
+
+  // Initialize the alias result.
+  Triple Trip(M->getTargetTriple());
+  TargetLibraryInfoImpl TLII(Trip);
+  TargetLibraryInfo TLI(TLII);
+  AAResults AA(TLI);
+  TypeBasedAAResult TBAAR;
+  AA.addAAResult(TBAAR);
+
+  // Initialize the alias set tracker for the @test function.
+  Function *Test = M->getFunction("test");
+  ASSERT_NE(Test, nullptr);
+  AliasSetTracker AST(AA);
+  for (auto &BB : *Test)
+    AST.add(BB);
+  // There should be 2 disjoint alias sets. 1 from each call. 
+  ASSERT_EQ((int)AST.getAliasSets().size(), 2);
+
+  // Directly test aliasesUnknownInst.
+  // Now every call instruction should only alias one alias set.
+  for (auto &Inst : *Test->begin()) {
+    bool FoundAS = false;
+    for (AliasSet &AS : AST) {
+      if (!AS.aliasesUnknownInst(&Inst, AA))
+        continue;
+      ASSERT_NE(FoundAS, true);
+      FoundAS = true;
+    }
+  }
+}

llvm/trunk/unittests/Analysis/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   Analysis
   AsmParser
   Core
   Support
   )
 
 add_llvm_unittest(AnalysisTests
   AliasAnalysisTest.cpp
+  AliasSetTrackerTest.cpp
   BlockFrequencyInfoTest.cpp
   BranchProbabilityInfoTest.cpp
   CallGraphTest.cpp
   CFGTest.cpp
   CGSCCPassManagerTest.cpp
   GlobalsModRefTest.cpp
   LazyCallGraphTest.cpp
   LoopInfoTest.cpp