[PATCH] D153464: Revert "[CaptureTracking] Ignore ephemeral values when determining pointer escapeness"

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 21 13:17:26 PDT 2023


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

This reverts commit 17fdaccccfad9b143e4aadbcdda7f645de127153 <https://reviews.llvm.org/rG17fdaccccfad9b143e4aadbcdda7f645de127153>.

Unfortunately the commit (D123162 <https://reviews.llvm.org/D123162>) introduced 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.

This is a mis-compile potentially impacting Clang builds with ThinLTO +
PGO.

I think as long as the call considered as ephemeral is not removed, we
need to be conservative. To address the correctness issue quickly, I
think we should revert the patch (as this patch does, it doens't revert
cleanly) and potentially consider what subset of ephemeral values we can
allow here or if we should clean up ephemeral calls sooner/avoid
inlining them.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153464

Files:
  llvm/include/llvm/Analysis/AliasAnalysis.h
  llvm/include/llvm/Analysis/CaptureTracking.h
  llvm/lib/Analysis/BasicAliasAnalysis.cpp
  llvm/lib/Analysis/CaptureTracking.cpp
  llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
  llvm/test/Transforms/DeadStoreElimination/assume.ll
  llvm/test/Transforms/PhaseOrdering/dse-ephemeral-value-captures.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D153464.533374.patch
Type: text/x-patch
Size: 12821 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230621/c7b72802/attachment.bin>


More information about the llvm-commits mailing list