[PATCH] D118727: [MemoryBuiltins][FIX] Adjust index type size properly wrt. AS casts

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert created this revision.
jdoerfert added reviewers: reames, spatel, arsenm, nikic.
Herald added subscribers: arphaman, bollu, hiraditya.
jdoerfert requested review of this revision.
Herald added a subscriber: wdng.
Herald added a project: LLVM.

We strip AS casts during the computation of size and offset in the
ObjectSizeOffsetVisitor. This is fine but we need to adjust the APInt
properly to avoid mismatches. This code ensures the caller of compute
sees APInts that match the index type size of the value passed to
compute, not the value result of the strip pointer cast.

NOTE: Ideas for tests are appreciated. My reproducer looks something
      like this: DSE runs on some stores to a pointer which is a
      select of two AS casts, on one side a GEP on the other an alloca.
      Unsure if/how I can trigger DSE with minimal example while causing
      it to run getObjSize (though I have not tried).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118727

Files:
  llvm/include/llvm/Analysis/MemoryBuiltins.h
  llvm/lib/Analysis/MemoryBuiltins.cpp


Index: llvm/lib/Analysis/MemoryBuiltins.cpp
===================================================================
--- llvm/lib/Analysis/MemoryBuiltins.cpp
+++ llvm/lib/Analysis/MemoryBuiltins.cpp
@@ -573,10 +573,34 @@
 }
 
 SizeOffsetType ObjectSizeOffsetVisitor::compute(Value *V) {
+  unsigned InitialIntTyBits = DL.getIndexTypeSizeInBits(V->getType());
+
+  // Stripping pointer casts can strip address space casts which can change the
+  // index type size. The invariant is that we use the value type to determine
+  // the index type size and if we stripped address space casts we "repair" the
+  // APInt as we pass it upwards so for the caller the APInt matches the type of
+  // the argument value V.
+  V = V->stripPointerCasts();
+
+  // Later we use the index type size and zero but it will match the type of the
+  // value that is passed to computeImpl.
   IntTyBits = DL.getIndexTypeSizeInBits(V->getType());
   Zero = APInt::getZero(IntTyBits);
 
-  V = V->stripPointerCasts();
+  if (InitialIntTyBits == IntTyBits)
+    return computeImpl(V);
+
+  // We stripped an address space cast that changed the index type size.
+  // Readjust it to match te argument's intex type size.
+  SizeOffsetType SOT = computeImpl(V);
+  if (knownSize(SOT) && !::CheckedZextOrTrunc(SOT.first, InitialIntTyBits))
+    return unknown();
+  if (knownOffset(SOT) && !::CheckedZextOrTrunc(SOT.second, InitialIntTyBits))
+    return unknown();
+  return SOT;
+}
+
+SizeOffsetType ObjectSizeOffsetVisitor::computeImpl(Value *V) {
   if (Instruction *I = dyn_cast<Instruction>(V)) {
     // If we have already seen this instruction, bail out. Cycles can happen in
     // unreachable code after constant propagation.
Index: llvm/include/llvm/Analysis/MemoryBuiltins.h
===================================================================
--- llvm/include/llvm/Analysis/MemoryBuiltins.h
+++ llvm/include/llvm/Analysis/MemoryBuiltins.h
@@ -221,6 +221,7 @@
   SizeOffsetType visitInstruction(Instruction &I);
 
 private:
+  SizeOffsetType computeImpl(Value *V);
   bool CheckedZextOrTrunc(APInt &I);
 };
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D118727.405062.patch
Type: text/x-patch
Size: 2090 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220201/f155149b/attachment.bin>