tabloid.adroit marked 2 inline comments as done.
tabloid.adroit added a comment.
Thanks for the review! Comments inline.
================
Comment at: lib/CodeGen/CGCall.cpp:1739
FuncAttrs.addAttribute("no-frame-pointer-elim", "true");
- FuncAttrs.addAttribute("no-frame-pointer-elim-non-leaf");
}
----------------
chandlerc wrote:
> This seems like an unrelated change?
The only user of "no-frame-pointer-elim-non-leaf" is
TargetOptions::DisableFramePointerElim where "no-frame-pointer-elim-non-leaf"
matters only if "no-frame-pointer-elim" is "false". This is to make it less
confusing.
================
Comment at: lib/Driver/ToolChains/Clang.cpp:592-595
+ if (Arg *A = Args.getLastArg(options::OPT_fno_omit_frame_pointer,
+ options::OPT_fomit_frame_pointer))
+ return A->getOption().matches(options::OPT_fno_omit_frame_pointer) ||
+ mustUseFramePointerForTarget(Triple);
----------------
chandlerc wrote:
> This doesn't correctly handle "last-flag-wins". Consider the case of
> `-mno-omit-leaf-frame-pointer -fomit-frame-pointer`. That should omit the
> leaf frame pointer, but if I read this correctly the logic here will use a
> leaf frame pointer.
Updated test with this case along with some other cases.
// RUN: %clang -### -S -Os -mno-omit-leaf-frame-pointer -fomit-frame-pointer %s
2>&1 | \
// RUN: FileCheck --check-prefix=OMIT-ALL5 %s
// OMIT-ALL5-NOT: "-mdisable-fp-elim"
// OMIT-ALL5-NOT: "-momit-leaf-frame-pointer"
This falls into lib/CodeGen/CGCall.cpp:1733, which causes
TargetOptions::DisableFramePointerElim returns false for all frames.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55915/new/
https://reviews.llvm.org/D55915
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
