This is an archive of the discontinued LLVM Phabricator instance.

Extend hasStoreToStackSlot with list of FI accesses.
ClosedPublic

Authored by sdesmalen on Aug 31 2018, 5:43 AM.

Details

Summary

For instructions that spill/fill to and from multiple frame-indices
in a single instruction, hasStoreToStackSlot and hasLoadFromStackSlot
should return an array of accesses, rather than just the first encounter
of such an access.

This better describes FI accesses for AArch64 (paired) LDP/STP
instructions.

Diff Detail

Event Timeline

sdesmalen created this revision.Aug 31 2018, 5:43 AM
MatzeB accepted this revision.Aug 31 2018, 1:35 PM

Good idea! LGTM.

lib/CodeGen/TargetInstrInfo.cpp
345

the type isn't obvious from immediate context, so I wouldn't use auto. Same in hasStoreToStackSlot.

This revision is now accepted and ready to land.Aug 31 2018, 1:35 PM
This revision was automatically updated to reflect the committed changes.
sdesmalen marked an inline comment as done.Sep 3 2018, 5:22 AM

I did a quick experiment to avoid storing the FrameIndex and get rid of the FrameAccess struct, by using this:

const PseudoSourceValue *PSV = MMO->getPseudoValue();
assert(PSV);
const FixedStackPseudoSourceValue *FSPSV = dyn_cast<FixedStackPseudoSourceValue>(PSV);
assert(FSPSV);
return FSPSV->getFrameIndex();

to get the FI, and tests seem to pass.

Although this works, I am wondering how much we can rely on this working properly. Any thoughts?

The code in hasLoadFromStackSlot filters out MMO's that have a pseudo source value of type Stack. If that function populates an array of only those MMOs, I think its safe to do a cast<FixedStackPseudoSourceValue>(mmo->getPseudoValue()) and get rid of the FrameAccess struct.