[BasicAliasAnalysis] Take into account operand bundles in the getModRefInfo function

[igor-laevsky]

Summary

In the getModRefInfo we were checking aliasing only for the actual call arguments. For cases when aliasing value was only used in the operand bundle we were returning NoModRef result (which is incorrect).

[reames]

LGTM

lib/Analysis/BasicAliasAnalysis.cpp

720 ↗

(On Diff #44988)

For clarity, it would be good to rename ArgNo to OperandNo since it can now iterate into the bundle operands not just the argument operands.

Alternatively, we could replace the induction variable with the CI-CS.data_operands_begin() like we did in CaptureTracking.

Doing this bit of cleanup in a separate change is fine.

734 ↗

(On Diff #44988)

A potential enhancement to this code would be to return MRI_Ref if the arguments which could alias the object are all readonly. This would be potentially useful for deopt bundles since all the arguments are implicitly read only.

If you decide to do this, it should definitely be a separate patch.

[igor-laevsky]

lib/Analysis/BasicAliasAnalysis.cpp

734 ↗

(On Diff #44988)

That makes sense. I'll send a separate patch.

llvm/trunk/lib/Analysis/BasicAliasAnalysis.cpp

if (const CallInst *CI = dyn_cast<CallInst>(CS.getInstruction()))
         return MRI_NoModRef;
 
   // If the pointer is to a locally allocated object that does not escape,
   // then the call can not mod/ref the pointer unless the call takes the pointer
   // as an argument, and itself doesn't capture it.
   if (!isa<Constant>(Object) && CS.getInstruction() != Object &&
       isNonEscapingLocalObject(Object)) {
     bool PassedAsArg = false;
-    unsigned ArgNo = 0;
-    for (ImmutableCallSite::arg_iterator CI = CS.arg_begin(), CE = CS.arg_end();
-         CI != CE; ++CI, ++ArgNo) {
+    unsigned OperandNo = 0;
+    for (auto CI = CS.data_operands_begin(), CE = CS.data_operands_end();
+         CI != CE; ++CI, ++OperandNo) {
       // Only look at the no-capture or byval pointer arguments.  If this
       // pointer were passed to arguments that were neither of these, then it
       // couldn't be no-capture.
       if (!(*CI)->getType()->isPointerTy() ||
-          (!CS.doesNotCapture(ArgNo) && !CS.isByValArgument(ArgNo)))
+          (!CS.doesNotCapture(OperandNo) && !CS.isByValArgument(OperandNo)))
         continue;
 
       // If this is a no-capture pointer argument, see if we can tell that it
       // is impossible to alias the pointer we're checking.  If not, we have to
       // assume that the call could touch the pointer, even though it doesn't
       // escape.
       AliasResult AR =
           getBestAAResults().alias(MemoryLocation(*CI), MemoryLocation(Object));

llvm/trunk/test/Transforms/DeadStoreElimination/operand-bundles.ll

+; RUN: opt < %s -basicaa -dse -S | FileCheck %s
+
+declare noalias i8* @malloc(i64) "malloc-like"
+
+declare void @foo()
+declare void @bar(i8*)
+
+define void @test() {
+  %obj = call i8* @malloc(i64 8)
+  store i8 0, i8* %obj
+  ; don't remove store. %obj should be treated like it will be read by the @foo.
+  ; CHECK: store i8 0, i8* %obj
+  call void @foo() ["deopt" (i8* %obj)]
+  ret void
+}
+
+define void @test1() {
+  %obj = call i8* @malloc(i64 8)
+  store i8 0, i8* %obj
+  ; CHECK: store i8 0, i8* %obj
+  call void @bar(i8* nocapture %obj)
+  ret void
+}
+
+define void @test2() {
+  %obj = call i8* @malloc(i64 8)
+  store i8 0, i8* %obj
+  ; CHECK-NOT: store i8 0, i8* %obj
+  call void @foo()
+  ret void
+}