[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