This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by ychen on Jan 5 2019, 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

ychen created this revision.Jan 5 2019, 12:15 AM
ychen retitled this revision from wip to Replace function attributes "no-frame-pointer-elim" and "no-frame-pointer-elim-non-leaf" with "frame-pointer".Jan 5 2019, 12:30 AM
ychen edited the summary of this revision. (Show Details)
ychen edited reviewers, added: chandlerc, rnk, t.p.northover; removed: mstorsjo.
ychen removed a reviewer: mstorsjo.Jan 5 2019, 12:30 AM
ychen edited the summary of this revision. (Show Details)Jan 5 2019, 12:33 AM
ychen edited the summary of this revision. (Show Details)Jan 5 2019, 1:38 AM
ychen edited the summary of this revision. (Show Details)
thegameg accepted this revision.Jan 7 2019, 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".
ychen updated this revision to Diff 181408.Jan 11 2019, 5:45 PM
  • update comment
ychen added a comment.Jan 11 2019, 5:56 PM

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

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