michaelplatings marked an inline comment as done.
michaelplatings added inline comments.
================
Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:151
+ const std::vector<StringRef> &Features) {
+ if (llvm::find(Features, "-fpregs") == Features.end())
+ return;
----------------
simon_tatham wrote:
> This whole patch hinges on the unspoken assumption that this is a correct way
> to test for the absence of FP registers, i.e. that `"-fpregs"` will
> //always// appear in `Features` if there are no FP registers.
>
> Could you add a comment stating that assumption explicitly and explaining why
> it's valid? In particular, why you're confident that the absence of FP
> registers might not sometimes be signalled by the lack of either `"-fpregs"`
> or `"+fpregs"` in that list.
>
> (In a quick look at the code I //think// you're right, but I'm not 100%
> confident, and if you show your working then people will be able to work out
> what went wrong later if things change.)
That code was relying on implementation details of the function that called it.
I've now changed checkARMFloatABI to require that the caller specifies whether
or not FP regs are available, and made that calculation clear in the calling
function.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D150902/new/
https://reviews.llvm.org/D150902
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
