This is an archive of the discontinued LLVM Phabricator instance.

Reapply Move "auto-init" instructions to the dominator of their users
ClosedPublic

Authored by serge-sans-paille on Apr 17 2023, 2:42 AM.

Details

Summary

Original patch (50b2a113db197a97f60ad2aace8b7382dc9b8c31) ignored the
fact that -ftrivial-auto-var-init could affect function parameters with
the sret attribute.
Just do not move instruction that don't affect alloca.
Also add missing test case for volatile instruction.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptApr 17 2023, 2:42 AM
serge-sans-paille requested review of this revision.Apr 17 2023, 2:42 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 17 2023, 2:42 AM
nikic added inline comments.Apr 17 2023, 5:40 AM
llvm/lib/Transforms/Utils/MoveAutoInit.cpp
56

Return MemoryLocation from this function and don't repeat the same in usersDominator?

I'd also replace stripInBoundsConstantsOffsets with getUnderlyingObject here. Unlikely to matter for auto-init in particular, but it's logically the right function.

Take @nikic comments into account.

nikic accepted this revision.Apr 24 2023, 5:42 AM

LGTM

llvm/lib/Transforms/Utils/MoveAutoInit.cpp
58

if (

67

const MemoryLocation &ML

llvm/test/Transforms/MoveAutoInit/sret.ll
13

Can you please convert the input IR to use opaque pointers? (Just run it through no-op opt -S).

This revision is now accepted and ready to land.Apr 24 2023, 5:42 AM
This revision was landed with ongoing or failed builds.Apr 24 2023, 9:13 AM
This revision was automatically updated to reflect the committed changes.