[BasicAA] Ignore @llvm.annotation when computing ModRefInfo.

[courbet]

Summary

@llvm.annotation is supposed to be ignored by code generation and
optimization:
https://llvm.org/docs/LangRef.html#llvm-annotation-intrinsic

However BasicAA considers that it may write to memory, so loads cannot
be moved across annotations.

BasicAA already ignores llvm.assume for this purpose. This change adds
llvm.annotation, llvm.var.annotation and llvm.ptr.annotation.

Fixes PR49638.

[courbet]

Rebase on test base to show diff.

[xbolva00]

llvm/lib/Analysis/BasicAliasAnalysis.cpp

941

Use switch (intrinsicCode) instead ifs?

[nikic]

Can we handle this by marking the intrinsic as InaccessibleMemOnly? Is there a reason it's modeled as arbitrarily writing now?

[courbet]

use switch as suggested

[courbet]

Thi sis similar to

In D98925#2636739, @nikic wrote:

Can we handle this by marking the intrinsic as InaccessibleMemOnly? Is there a reason it's modeled as arbitrarily writing now?

If I read the comment correctly it's to guarantee that control dependencies are respected.

[courbet]

fix conjugation.

[courbet]

Retiring this in favor of D99022.

llvm/lib/Analysis/BasicAliasAnalysis.cpp

if (auto *Inst = dyn_cast<AnyMemCpyInst>(Call)) {
     ModRefInfo rv = ModRefInfo::NoModRef;
     if (SrcAA != NoAlias)
       rv = setRef(rv);
     if (DestAA != NoAlias)
       rv = setMod(rv);
     return rv;
   }
 
-  // While the assume intrinsic is marked as arbitrarily writing so that
-  // proper control dependencies will be maintained, it never aliases any
+  if (const IntrinsicInst *II = dyn_cast<IntrinsicInst>(Call)) {
+    switch (II->getIntrinsicID()) {
+    // While these intrinsics are marked as arbitrarily writing so that
+    // proper control dependencies will be maintained, they never aliases any
     // particular memory location.
-  if (isIntrinsicCall(Call, Intrinsic::assume))
+    case Intrinsic::annotation:
+    case Intrinsic::assume:
+    case Intrinsic::ptr_annotation:
+    case Intrinsic::var_annotation:
       return ModRefInfo::NoModRef;
-
-  // Like assumes, guard intrinsics are also marked as arbitrarily writing so
-  // that proper control dependencies are maintained but they never mods any
-  // particular memory location.
+    // Like assumes, guard intrinsics are also marked as arbitrarily writing
+    // so that proper control dependencies are maintained but they never mods
+    // any particular memory location.
     //
-  // *Unlike* assumes, guard intrinsics are modeled as reading memory since the
-  // heap state at the point the guard is issued needs to be consistent in case
-  // the guard invokes the "deopt" continuation.
-  if (isIntrinsicCall(Call, Intrinsic::experimental_guard))
-    return ModRefInfo::Ref;
+    // *Unlike* assumes, guard intrinsics are modeled as reading memory since
+    // the heap state at the point the guard is issued needs to be consistent
+    // in case the guard invokes the "deopt" continuation.
+    case Intrinsic::experimental_guard:
     // The same applies to deoptimize which is essentially a guard(false).
-  if (isIntrinsicCall(Call, Intrinsic::experimental_deoptimize))
-    return ModRefInfo::Ref;
-
-  // Like assumes, invariant.start intrinsics were also marked as arbitrarily
-  // writing so that proper control dependencies are maintained but they never
-  // mod any particular memory location visible to the IR.
+    case Intrinsic::experimental_deoptimize:
+    // Like assumes, invariant.start intrinsics were also marked as
+    // arbitrarily writing so that proper control dependencies are maintained
+    // but they never mod any particular memory location visible to the IR.
     // *Unlike* assumes (which are now modeled as NoModRef), invariant.start
     // intrinsic is now modeled as reading memory. This prevents hoisting the
     // invariant.start intrinsic over stores. Consider:
     // *ptr = 40;
     // *ptr = 50;
     // invariant_start(ptr)
     // int val = *ptr;
     // print(val);
     //
     // This cannot be transformed to:
     //
     // *ptr = 40;
     // invariant_start(ptr)
     // *ptr = 50;
     // int val = *ptr;
     // print(val);
     //
     // The transformation will cause the second store to be ignored (based on
     // rules of invariant.start)  and print 40, while the first program always
     // prints 50.
-  if (isIntrinsicCall(Call, Intrinsic::invariant_start))
+    case Intrinsic::invariant_start:
       return ModRefInfo::Ref;
+    default:
+      break;
+    }
+  }
 
   // The AAResultBase base class has some smarts, lets use them.
   return AAResultBase::getModRefInfo(Call, Loc, AAQI);
 }
 
 ModRefInfo BasicAAResult::getModRefInfo(const CallBase *Call1,
                                         const CallBase *Call2,
                                         AAQueryInfo &AAQI) {

llvm/test/Transforms/InstCombine/annotation-intrinsic.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt -instcombine %s -S -o - | FileCheck %s
 
 ; This tests that llvm.annotation does not prevent load combining.
 
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-grtev4-linux-gnu"
 
 declare i32 @llvm.annotation.i32(i32, i8*, i8*, i32) #1
 
 define dso_local i32 @annotated(i32* %c) local_unnamed_addr #0 {
 ; CHECK-LABEL: @annotated(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[TMP0:%.*]] = load i32, i32* [[C:%.*]], align 4
 ; CHECK-NEXT:    [[TMP1:%.*]] = call i32 @llvm.annotation.i32(i32 [[TMP0]], i8* undef, i8* undef, i32 undef)
-; CHECK-NEXT:    [[TMP2:%.*]] = load i32, i32* [[C]], align 4
-; CHECK-NEXT:    [[ADD:%.*]] = add nsw i32 [[TMP1]], [[TMP2]]
+; CHECK-NEXT:    [[ADD:%.*]] = add nsw i32 [[TMP1]], [[TMP0]]
 ; CHECK-NEXT:    ret i32 [[ADD]]
 ;
 entry:
   %0 = load i32, i32* %c, align 4
   %1 = call i32 @llvm.annotation.i32(i32 %0, i8* undef, i8* undef, i32 undef)
   %2 = load i32, i32* %c, align 4
   %add = add nsw i32 %1, %2
   ret i32 %add
 }
 
 attributes #0 = { nofree nounwind uwtable willreturn mustprogress }