[llvm] 4d59ffb - [InstCombine] Simplify separate_storage assumptions

[Nikita Popov via llvm-commits]

Author: David Goldblatt
Date: 2023-03-03T13:26:29+01:00
New Revision: 4d59ffb0d19a40c974ce930e8e9336d3df4d2dd3

URL: https://github.com/llvm/llvm-project/commit/4d59ffb0d19a40c974ce930e8e9336d3df4d2dd3
DIFF: https://github.com/llvm/llvm-project/commit/4d59ffb0d19a40c974ce930e8e9336d3df4d2dd3.diff

LOG: [InstCombine] Simplify separate_storage assumptions

Before this change, we call getUnderlyingObject on each separate_storage
operand on every alias() call (potentially requiring lots of pointer
chasing). Instead, we rewrite the assumptions in instcombine to do this
pointer-chasing once.

We still leave the getUnderlyingObject calls in alias(), just expecting
them to be no-ops much of the time. This is relatively fast (just a
couple dyn_casts with no pointer chasing) and avoids making alias
analysis results depend on whether or not instcombine has been run.

Differential Revision: https://reviews.llvm.org/D144933

Added: 
    llvm/test/Transforms/InstCombine/assume-separate_storage.ll

Modified: 
    llvm/lib/Analysis/BasicAliasAnalysis.cpp
    llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Analysis/BasicAliasAnalysis.cpp b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
index b677eae2c76c..f806e373d925 100644
--- a/llvm/lib/Analysis/BasicAliasAnalysis.cpp
+++ b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
@@ -1516,6 +1516,8 @@ AliasResult BasicAAResult::aliasCheck(const Value *V1, LocationSize V1Size,
           assert(OBU.Inputs.size() == 2);
           const Value *Hint1 = OBU.Inputs[0].get();
           const Value *Hint2 = OBU.Inputs[1].get();
+          // This is often a no-op; instcombine rewrites this for us. No-op
+          // getUnderlyingObject calls are fast, though.
           const Value *HintO1 = getUnderlyingObject(Hint1);
           const Value *HintO2 = getUnderlyingObject(Hint2);
 

diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
index b7fe9ff239a3..8b8514194810 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -2516,6 +2516,27 @@ Instruction *InstCombinerImpl::visitCallInst(CallInst &CI) {
       // TODO: apply range metadata for range check patterns?
     }
 
+    // Separate storage assumptions apply to the underlying allocations, not any
+    // particular pointer within them. When evaluating the hints for AA purposes
+    // we getUnderlyingObject them; by precomputing the answers here we can
+    // avoid having to do so repeatedly there.
+    for (unsigned Idx = 0; Idx < II->getNumOperandBundles(); Idx++) {
+      OperandBundleUse OBU = II->getOperandBundleAt(Idx);
+      if (OBU.getTagName() == "separate_storage") {
+        assert(OBU.Inputs.size() == 2);
+        auto MaybeSimplifyHint = [&](const Use &U) {
+          Value *Hint = U.get();
+          // Not having a limit is safe because InstCombine removes unreachable
+          // code.
+          Value *UnderlyingObject = getUnderlyingObject(Hint, /*MaxLookup*/ 0);
+          if (Hint != UnderlyingObject)
+            replaceUse(const_cast<Use &>(U), UnderlyingObject);
+        };
+        MaybeSimplifyHint(OBU.Inputs[0]);
+        MaybeSimplifyHint(OBU.Inputs[1]);
+      }
+    }
+
     // Convert nonnull assume like:
     // %A = icmp ne i32* %PTR, null
     // call void @llvm.assume(i1 %A)

diff  --git a/llvm/test/Transforms/InstCombine/assume-separate_storage.ll b/llvm/test/Transforms/InstCombine/assume-separate_storage.ll
new file mode 100644
index 000000000000..8fa8c3e80786
--- /dev/null
+++ b/llvm/test/Transforms/InstCombine/assume-separate_storage.ll
@@ -0,0 +1,50 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -S -passes=instcombine < %s | FileCheck %s
+
+declare void @llvm.assume(i1 noundef)
+
+; Just something to let us check that separate_storage bundles don't break
+; anything when given an operand that's not an instruction to fold.
+ at some_global = global i32 777
+
+define void @simple_folding(ptr %a, ptr %b) {
+; CHECK-LABEL: @simple_folding(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    call void @llvm.assume(i1 true) [ "separate_storage"(ptr [[A:%.*]], ptr [[B:%.*]]) ]
+; CHECK-NEXT:    ret void
+;
+entry:
+  %p1 = getelementptr i8, ptr %a, i64 123
+  %p2 = getelementptr i8, ptr %b, i64 777
+  call void @llvm.assume(i1 1) ["separate_storage"(ptr %p1, ptr %p2)]
+  ret void
+}
+
+define i64 @folds_removed_operands(ptr %a, ptr %b, i64 %n1, i64 %n2) {
+; CHECK-LABEL: @folds_removed_operands(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[REASS_ADD:%.*]] = shl i64 [[N2:%.*]], 1
+; CHECK-NEXT:    [[Y:%.*]] = add i64 [[REASS_ADD]], [[N1:%.*]]
+; CHECK-NEXT:    call void @llvm.assume(i1 true) [ "separate_storage"(ptr [[A:%.*]], ptr [[B:%.*]]) ]
+; CHECK-NEXT:    ret i64 [[Y]]
+;
+entry:
+  ; Ordinarily, n1 + n2 + n2 would get canonicalized into n1 + (n2 << 1) unless
+  ; there's another use of n1 + n2. Make sure that we remember to put removed
+  ; arguments to separate_storage bundles back on the worklist.
+  %x = add i64 %n1, %n2
+  %y = add i64 %x, %n2
+  %p1 = getelementptr i8, ptr %a, i64 %x
+  call void @llvm.assume(i1 1) ["separate_storage"(ptr %p1, ptr %b)]
+  ret i64 %y
+}
+
+define void @handles_globals(ptr %a) {
+; CHECK-LABEL: @handles_globals(
+; CHECK-NEXT:    call void @llvm.assume(i1 true) [ "separate_storage"(ptr [[A:%.*]], ptr @some_global) ]
+; CHECK-NEXT:    ret void
+;
+  %derived = getelementptr i8, ptr @some_global, i65 3
+  call void @llvm.assume(i1 1) ["separate_storage"(ptr %a, ptr %derived)]
+  ret void
+}