[PATCH] D60697: [ARM] Allow "-march=foo+fp" to vary with foo.

Tue Jun 4 07:27:42 PDT 2019

SjoerdMeijer updated this revision to Diff 202934.
SjoerdMeijer added a comment.

Hi Oliver, thanks for your comments!

This was the easy one, they have been added:

> I also don't see any tests for the negated forms of either feature.

The trouble begun with this:

> +fp.dp, but the FPU is already double-precision
>  +fp.dp, but no double-precision FPU exists (are there any FPUs which cause this?)
>  +[no]fp or +[no]fp.dp for a CPU/arch which doesn't have any FPUs.

Because I found that basically none of this worked. The main reason was that we were always passing `generic`. To address that we at least have a chance of seeing a sensible CPU name, I have swapped the order of parsing -march and -mcpu. I.e., we parse -mcpu first, and pass that to `checkARMArchName`, which will eventually call `appendArchExtFeatures`. I think that makes more sense when we use the CPUname to query `getDefaultFPU`.

Then about the more fancy diagnostics (e.g. "fp.dp, but the FPU is already double-precision"): I've removed any attempt to throw clever diagnostics. I don't think, in general, that we provide this kind of service level. I.e., we need to do a lot more work here to avoid a meaningless, confusing, and thus useless  "--march=... not supported" error message when we provide +fp.dp on the -march when e.g. the CPU already enabled this.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60697/new/

https://reviews.llvm.org/D60697

Files:
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/lib/Driver/ToolChains/Arch/ARM.h
  clang/test/Driver/armv8.1m.main.c
  clang/test/Driver/armv8.1m.main.s
  llvm/include/llvm/Support/ARMTargetParser.h
  llvm/lib/Support/ARMTargetParser.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D60697.202934.patch
Type: text/x-patch
Size: 15928 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190604/119a08f1/attachment.bin>