[PATCH] D153577: [CaptureTracking] Consider ephemeral calls accessing memory as capturing.

[Florian Hahn via Phabricator via llvm-commits]

fhahn created this revision.
fhahn added reviewers: asbirlea, nikic, reames, aeubanks.
Herald added subscribers: StephenFan, hiraditya.
Herald added a project: All.
fhahn requested review of this revision.
Herald added a project: LLVM.

Consider ephemeral calls reading or writing memory as capturing to avoid
memory optimizations that introduce new UB after inlining such calls.

This fixes a mis-compile (see
llvm/test/Transforms/PhaseOrdering/dse-ephemeral-value-captures.ll). The
issue is roughly:

1. There's a call to a readonly function that takes a pointer argument and its  result is only used by an assume, so the call is considered ephemeral and not capturing.
2. Because the call doesn't capture the pointer, it's considered to not read the pointer, causing DSE to remove the store before the call.
3. Now the called function gets inlined and there's now a load from the pointer, but the initializing store has been removed
4. This leads to SROA replacing the load with undef, which will cause the function to get folded to unreachable by subsequent optimizations.

Alternative to D153464 <https://reviews.llvm.org/D153464>


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153577

Files:
  llvm/lib/Analysis/CaptureTracking.cpp
  llvm/test/Transforms/PhaseOrdering/dse-ephemeral-value-captures.ll


Index: llvm/test/Transforms/PhaseOrdering/dse-ephemeral-value-captures.ll
===================================================================
--- llvm/test/Transforms/PhaseOrdering/dse-ephemeral-value-captures.ll
+++ llvm/test/Transforms/PhaseOrdering/dse-ephemeral-value-captures.ll
@@ -26,7 +26,13 @@
 define i32 @test() {
 ; CHECK-LABEL: define i32 @test() {
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    unreachable
+; CHECK-NEXT:    br label [[THEN_I:%.*]]
+; CHECK:       then.i:
+; CHECK-NEXT:    [[RES_I:%.*]] = call i1 @cond()
+; CHECK-NEXT:    br label [[CHECK_COND_EXIT:%.*]]
+; CHECK:       check_cond.exit:
+; CHECK-NEXT:    call void @llvm.assume(i1 [[RES_I]])
+; CHECK-NEXT:    ret i32 0
 ;
 entry:
   %a = alloca i32, align 4
Index: llvm/lib/Analysis/CaptureTracking.cpp
===================================================================
--- llvm/lib/Analysis/CaptureTracking.cpp
+++ llvm/lib/Analysis/CaptureTracking.cpp
@@ -89,7 +89,9 @@
       if (isa<ReturnInst>(U->getUser()) && !ReturnCaptures)
         return false;
 
-      if (EphValues.contains(U->getUser()))
+      // Consider ephemeral calls reading or writing memory as capturing to avoid memory optimizations that may result in UB after inlining such calls.
+      if (EphValues.contains(U->getUser()) &&
+          !cast<Instruction>(U->getUser())->mayReadOrWriteMemory())
         return false;
 
       LLVM_DEBUG(dbgs() << "Captured by: " << *U->getUser() << "\n");
@@ -181,7 +183,8 @@
       if (isa<ReturnInst>(I) && !ReturnCaptures)
         return false;
 
-      if (EphValues.contains(I))
+      // Consider ephemeral calls reading or writing memory as capturing to avoid memory optimizations that may result in UB after inlining such calls.
+      if (EphValues.contains(I) && !I->mayReadOrWriteMemory())
         return false;
 
       if (!EarliestCapture)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D153577.533696.patch
Type: text/x-patch
Size: 1848 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230622/83e871d8/attachment.bin>