[PATCH] D136146: [Clang][LoongArch] Handle -march/-m{single,double,soft}-float/-mfpu options

[Fangrui Song via Phabricator via llvm-commits]

MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

In D136146#3911137 <https://reviews.llvm.org/D136146#3911137>, @SixWeining wrote:

> Sorry for the late reply.
>
> Should we choose not to implement the `-mfpu=` option which is not mandatory?

OK, I accept the `-mfpu=` different from `-m*-float`. If the patch does not implement the required semantics, deferring `-mfpu=` implementation may be good.
We can also check how many pieces of software actually use `-mfpu=` and some unneeded uses may be removed?



================
Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:696
+def err_drv_loongarch_invalid_mfpu_EQ : Error<
+  "invalid argument '%0' to -mfpu=; must be one of: 64, 32, 0, none">;
 }
----------------
`0, none (alias for 0)` or `none, 0 (alias for none)`

Ideally only one spelling is retained... No need to implement an alias if -mfpu= itself is niche.


================
Comment at: llvm/lib/Support/LoongArchTargetParser.cpp:46
+        if ((A.Features & F.Kind) == F.Kind && F.Kind != FK_INVALID) {
+          Features.push_back(F.Name);
+        }
----------------
SixWeining wrote:
> MaskRay wrote:
> > delete braces
> OK.
`for (const auto A : AllArchs)` needs braces since the nested `if (A.Name == Arch) {` has braces.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136146