vhscampos marked 2 inline comments as done.
vhscampos added inline comments.
================
Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:292-297
+ auto checkFPDisabledInArchName = [](const StringRef &ArchName) {
+ SmallVector<StringRef, 8> Split;
+ ArchName.split(Split, '+', -1, false);
+ return llvm::any_of(
+ Split, [](const StringRef &Extension) { return Extension == "nofp"; });
+ };
----------------
vhscampos wrote:
> chill wrote:
> > chill wrote:
> > > Wouldn't just looking for the substring do the job?
> > >
> > > Also need to handle `-mcpu=...+nofp`.
> > >
> > > We already "parse" the arguments to `-march=` and `-mcpu=` (and `-mfpu=`)
> > > earlier, it seems to me we
> > > could note the `+nofp` and `+nofp.dp` earlier. (TBH, it isn't immediately
> > > obvious to me how to untangle this mess).
> > >
> > Hmm, actually, `+nofp.dp` should not disable the FPU, I think.
> Just looking for the substring might be sufficient indeed.
>
> Yes, we already do `-march`/`-mcpu` parsing a bit earlier. However, this
> parsing and the following handling of it is done deeper in the call stack. I
> wondered about ways to propagate this information back to this point here
> (e.g. adding one more by-ref argument that is set by the first round of
> parsing), but I don't feel confident to back it up.
>
> Are you okay with me just changing it to a substring search?
Actually it may be better to keep the string splitting method. The search
required here must be whole-word, as to flag up "+nofp", but not "+nofp.dp". It
can be done with less code using the current list of tokens as opposed to using
substring search, followed by a "is it whole-word?" check.
================
Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:296
+ return llvm::any_of(
+ Split, [](const StringRef &Extension) { return Extension == "nofp"; });
+ };
----------------
DavidSpickett wrote:
> I would check what this does:
> $ ./bin/clang -target arm-arm-none-eabi /tmp/test.c -c
> -march=armv8.1-a+nofp+fp -###
>
> I'm not sure if by this point we've resolved "+nofp+fp" to simply "+fp".
> (perhaps you'd end up with a bunch of "-<feature>" followed by the same as
> "+<feature>". Either way I'd expect +fp to win.
Good catch. I will be sure to cover this case.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82948/new/
https://reviews.llvm.org/D82948
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
