[PATCH] D144933: [instcombine] Simplify separate_storage hints.

[David Goldblatt via Phabricator via llvm-commits]

davidtgoldblatt updated this revision to Diff 501542.
davidtgoldblatt added a comment.

Nits.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144933/new/

https://reviews.llvm.org/D144933

Files:
  llvm/lib/Analysis/BasicAliasAnalysis.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
  llvm/test/Transforms/InstCombine/assume-separate_storage.ll


Index: llvm/test/Transforms/InstCombine/assume-separate_storage.ll
===================================================================
--- /dev/null
+++ 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
+}
Index: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
===================================================================
--- llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -2516,6 +2516,27 @@
       // 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)
Index: llvm/lib/Analysis/BasicAliasAnalysis.cpp
===================================================================
--- llvm/lib/Analysis/BasicAliasAnalysis.cpp
+++ llvm/lib/Analysis/BasicAliasAnalysis.cpp
@@ -1516,6 +1516,8 @@
           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);
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D144933.501542.patch
Type: text/x-patch
Size: 4256 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230301/201316cd/attachment.bin>