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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
