[PATCH] D82948: [Driver][ARM] Disable unsupported features when nofp arch extension is used

Thu Jul 23 05:11:15 PDT 2020

DavidSpickett marked an inline comment as done.
DavidSpickett added a comment.

LGTM, I'll let @chill give the final word.

================
Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:476
+    // -mfpu=none, -march=armvX+nofp or -mcpu=X+nofp is *very* similar to
+    // -mfloat-abi=soft, only that it should not disable MVE-I.
     Features.insert(Features.end(),
----------------
DavidSpickett wrote:
> chill wrote:
> > DavidSpickett wrote:
> > > Why not disable MVE-I? I assume because it's integer only but then why does -mfloat-abi=soft disable it?
> > > 
> > > If possible add a regression test for this. In general a test like the bf16 test below, but for all the listed extensions would help. Perhaps it makes more sense to add a driver test that looks for the "-<ext>" bits in the -### output instead of doing each extension on its own.
> > > Why not disable MVE-I?
> > 
> > After MVE, "FPU" registers are a separate entity from the FPU.
> > 
> > `-mfpu=none`/`+nofp` disable the FPU. MVE-I does not require an FPU.
> > `-mfloat-abi=soft` disables both the FPU instructions and the FPU registers.
> > MVE-I requires "FPU" registers.
> > 
> > It's possible to define different semantics, but this is the one we agreed with GCC.
> Got it. @vhscampos  can you add that to the comment?
Thanks, lgtm.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82948