SixWeining added a comment.
Sorry for the late reply.
Should we choose not to implement the `-mfpu=` option which is not mandatory?
================
Comment at: clang/lib/Driver/ToolChains/Arch/LoongArch.cpp:31
+ options::OPT_msingle_float,
+ options::OPT_msoft_float)) {
+ if (A->getOption().matches(options::OPT_mdouble_float))
----------------
MaskRay wrote:
> This is strange. Other architectures using -msoft-float makes it orthogonal
> to -mdouble-float...
>
> Instead of having -mdouble-float/-msingle-float, can you just use
> -mfloat-abi=?
Sorry for the late reply.
Similar to @xry111's explanation below, `-m{double,single,soft}-float` are
different from `-mfpu={64,32,0,none}`.
`-m{double,single,soft}-float` affect both codegen-ed instructions and the ABI
while `-mfpu={64,32,0,none}` only affect the codegen-ed instructions.
That is to say:
* `-mdouble-float` implies `-mfpu=64` and `-mabi={lp64d,ilp32d}`;
* `-msingle-float` implies `-mfpu=32` and `-mabi={lp64f,ilp32f}`;
* `-msoft-float` implies `-mfpu={0,none}` and `-mabi={lp64s,ilp32s}`.
The `-mfpu={64,32,0,none}` clang option is like the `--mattr=+{d,f}` llc
option. And the `-mabi=` clang option is like the `--target-abi=` llc option.
But I have to admit this introduce some complexity to clang implementation
because we have to take care of the order these options appear on the command
line. The doc says:
> As a general rule, the effect of all LoongArch-specific compiler options that
> are given for one compiler invocation should be as if they are processed in
> the order they appear on the command line. The only exception to this rule is
> -m*-float: their configuration of floating-point instruction set and calling
> convention will not be changed by subsequent options other than -m*-float.
================
Comment at: clang/lib/Driver/ToolChains/Arch/LoongArch.cpp:44
+ // Select abi based on -mfpu=xx.
+ if (const Arg *A = Args.getLastArg(options::OPT_mfpu_EQ)) {
----------------
xry111 wrote:
> MaskRay wrote:
> > It's better to stick with just one canonical spelling. Is there time to
> > remove support for one set of options? When `-mfpu=64 -msoft-float` is
> > specified, is a -Wunused-command-line-argument warning expected?
> According to the doc, the semantics of -mfpu=64 and -mdouble-float are not
> exactly same. `-mfpu=64 -mabi=lp64s` will allow the compiler to generate
> 64-bit floating-point instructions but keep LP64S calling convention.
> `-mdouble-float -mabi=lp64s` will be same as `-mdouble-float -mabi=lp64d`
> (with a warning emitted, maybe).
The doc says that the implementation of `-mfpu` is not mandatory. So maybe we
can choose not to implement it? But I'm not sure whether it has been used by
any programs.
> -mdouble-float -mabi=lp64s will be same as -mdouble-float -mabi=lp64d (with a
> warning emitted, maybe).
Yes, I think so. GCC indeed emits a warning. We also should emit one.
================
Comment at: clang/test/Driver/loongarch-march-error.c:1
+// RUN: not %clang --target=loongarch64 -march=loongarch -fsyntax-only %s 2>&1
\
+// RUN: | FileCheck --check-prefix=LOONGARCH %s
----------------
MaskRay wrote:
> The more common style is to place `|` in the end. The idea is that even
> without `\`, after typing the first line in a shell, it will wait for the
> second line.
OK. But seems both styles are used in the code base. -;)
================
Comment at: clang/test/Driver/loongarch-march.c:12
+// CC1-LOONGARCH64: "-target-feature" "+64bit"
+// CC1-LOONGARCH64-SAME: {{^}} "-target-feature" "+f"
+// CC1-LOONGARCH64-SAME: {{^}} "-target-feature" "+d"
----------------
MaskRay wrote:
> Check target-feature options on one line
OK.
================
Comment at: llvm/lib/Support/LoongArchTargetParser.cpp:46
+ if ((A.Features & F.Kind) == F.Kind && F.Kind != FK_INVALID) {
+ Features.push_back(F.Name);
+ }
----------------
MaskRay wrote:
> delete braces
OK.
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
