Page MenuHomePhabricator

Replace function attributes "no-frame-pointer-elim" and "no-frame-pointer-elim-non-leaf" with "frame-pointer"
ClosedPublic

Authored by tabloid.adroit on Sat, Jan 5, 12:15 AM.

Details

Summary

Part of the effort to refactoring frame pointer code generation. We used to use two function attributes "no-frame-pointer-elim" and "no-frame-pointer-elim-non-leaf" to represent three kinds of frame pointer usage: (all) frames use frame pointer, (non-leaf) frames use frame pointer, (none) frame use frame pointer. This CL makes the idea explicit by using only one enum function attribute "frame-pointer"

Option "-frame-pointer=" replaces "-disable-fp-elim" for tools such as llc.

"no-frame-pointer-elim" and "no-frame-pointer-elim-non-leaf" are still supported for easy migration to "frame-pointer".

tests are mostly updated with

// replace command line args ‘-disable-fp-elim=false’ with ‘-frame-pointer=none’
grep -iIrnl '\-disable-fp-elim=false' * | xargs sed -i '' -e "s/-disable-fp-elim=false/-frame-pointer=none/g"

// replace command line args ‘-disable-fp-elim’ with ‘-frame-pointer=all’
grep -iIrnl '\-disable-fp-elim' * | xargs sed -i '' -e "s/-disable-fp-elim/-frame-pointer=all/g"

related:
https://reviews.llvm.org/D55915

Diff Detail

Repository
rL LLVM

Event Timeline

tabloid.adroit created this revision.Sat, Jan 5, 12:15 AM
tabloid.adroit retitled this revision from wip to Replace function attributes "no-frame-pointer-elim" and "no-frame-pointer-elim-non-leaf" with "frame-pointer".Sat, Jan 5, 12:30 AM
tabloid.adroit edited the summary of this revision. (Show Details)
tabloid.adroit edited reviewers, added: chandlerc, rnk, t.p.northover; removed: mstorsjo.
tabloid.adroit edited the summary of this revision. (Show Details)Sat, Jan 5, 12:33 AM
tabloid.adroit edited the summary of this revision. (Show Details)Sat, Jan 5, 1:38 AM
tabloid.adroit edited the summary of this revision. (Show Details)
thegameg accepted this revision.Mon, Jan 7, 6:54 AM
thegameg added a reviewer: thegameg.

This LGTM with one small comment change, thanks! You might want to add to the commit message the fact that the target now overrides the function attribute, which could change the behavior for (out of tree) targets.

include/llvm/CodeGen/TargetFrameLowering.h
211 ↗(On Diff #180361)

I would say something like:

Return true if the target wants to keep the frame pointer regardless of the function attribute "frame-pointer".
  • update comment

Thanks! Comment addressed. Could you help me check in this?

This revision was not accepted when it landed; it landed in state Needs Review.Mon, Jan 14, 3:01 AM
This revision was automatically updated to reflect the committed changes.