This is an archive of the discontinued LLVM Phabricator instance.

[ConstFold] Simplify a load's GEP operand through local aliases
ClosedPublic

Authored by aeubanks on Apr 22 2021, 12:26 PM.

Details

Summary

MSVC-style RTTI produces loads through a GEP of a local alias which
itself is a GEP. Currently we aren't able to devirtualize any virtual
calls when MSVC RTTI is enabled.

This patch attempts to simplify a load's GEP operand by calling
SymbolicallyEvaluateGEP() with an option to look through local aliases.

Diff Detail

Event Timeline

aeubanks created this revision.Apr 22 2021, 12:26 PM
aeubanks requested review of this revision.Apr 22 2021, 12:26 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 22 2021, 12:26 PM
rnk added a comment.Apr 26 2021, 6:49 PM

Should we go forward with something in the direction of D101103 instead, or is this still the way forward?

llvm/lib/IR/Value.cpp
624–625 ↗(On Diff #339756)

Do we need checks for non-interposability on the alias and aliasee here?

aeubanks updated this revision to Diff 340992.Apr 27 2021, 2:42 PM

properly handle non-interposable aliases, add tests

aeubanks updated this revision to Diff 345237.May 13 2021, 11:32 AM

rebase
add comments

rnk added inline comments.May 25 2021, 11:52 AM
llvm/lib/Analysis/ConstantFolding.cpp
874–881

We talked about moving the alias handling here rather than adding an IR helper.

aeubanks updated this revision to Diff 348054.May 26 2021, 12:37 PM

move everything out of Value into ConstantFolding.cpp

aeubanks edited the summary of this revision. (Show Details)May 26 2021, 12:37 PM
rnk accepted this revision.May 26 2021, 4:19 PM

lgtm

This revision is now accepted and ready to land.May 26 2021, 4:19 PM
This revision was landed with ongoing or failed builds.May 27 2021, 4:04 PM
This revision was automatically updated to reflect the committed changes.
nikic added inline comments.Oct 2 2021, 1:58 PM
llvm/lib/Analysis/ConstantFolding.cpp
877

This code uses a different check than the top-level case (https://github.com/llvm/llvm-project/blob/26223af256bb8f0aa1a82989882c81ffae44c6d1/llvm/lib/Analysis/ConstantFolding.cpp#L682-L684). Namely it also checks that the base object is not interposable. What's the reason for the difference? I'd expect both of them to use the same conditions.

aeubanks added inline comments.Oct 4 2021, 5:59 PM
llvm/lib/Analysis/ConstantFolding.cpp
877

See the discussions in https://reviews.llvm.org/D99240, https://reviews.llvm.org/D65118, https://bugs.llvm.org/show_bug.cgi?id=27866.

I keep getting confused by these explanations. But as I understand it right now, we are only allowed to replace an alias with its aliasee if both are not interposable (but we may not want to do this anyway). However, we can look through an alias to get the aliasee's contents if the alias is not interposable. So this patch is probably overly restrictive.