This is an archive of the discontinued LLVM Phabricator instance.

Move SPAdj logic from PEI into the targets (No functionality change.)
ClosedPublic

Authored by mkuper on Jan 7 2015, 6:55 AM.

Details

Summary

The back-story here is that PEI tries to keep track of how much starting or ending a call sequence adjusts the stack pointer, so that it can resolve frame-index references.
Currently, it takes a very simplistic view of how SP adjustments are done - both FrameStartOpcode and FrameDestroyOpcode adjust it exactly by the amount written in its first argument.
This is in fact false for many targets (e.g. due to stack re-alignment, or because it may want to adjust the stack pointer in multiple steps).

So why don't things break? Because most targets (the only in-tree exception I could find is 32-bit ARM) rely on being able to simplify the call frame pseudo-instructions earlier, so this code is never hit. This assumption is enforced by an "assert(SPAdj == 0 && "Unexpected")" in eliminateFrameIndex().

I want to start using it on x86 (to make the mov -> push transform work, http://reviews.llvm.org/D6789 will depend on this), and that means delegating the decision to the target, so that x86 can return the actual adjustment value.

Diff Detail

Event Timeline

mkuper updated this revision to Diff 17856.Jan 7 2015, 6:55 AM
mkuper retitled this revision from to Move SPAdj logic from PEI into the targets (No functionality change.).
mkuper updated this object.
mkuper edited the test plan for this revision. (Show Details)
mkuper added reviewers: rafael, rnk, t.p.northover.
mkuper added a subscriber: Unknown Object (MLST).
hfinkel accepted this revision.Jan 7 2015, 8:18 AM
hfinkel added a reviewer: hfinkel.
hfinkel added a subscriber: hfinkel.

LGTM.

This revision is now accepted and ready to land.Jan 7 2015, 8:18 AM
rafael edited edge metadata.Jan 7 2015, 8:39 AM

just nits

include/llvm/Target/TargetInstrInfo.h
112

don't repeat the name in the comment.

lib/CodeGen/TargetInstrInfo.cpp
648

Don't repeat the comment in the implementation.

mkuper added a comment.Jan 8 2015, 1:06 AM

Thanks Hal, Rafael.

Rafael, re the nits - I was just following the convention of the surrounding code, which adds the name and repeats the comments.
Will make the changes. :-)

This revision was automatically updated to reflect the committed changes.
majnemer added inline comments.
llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp
31 ↗(On Diff #17890)

Please sort this before llvm/Target/TargetLowering.h