[RISCV] Pass -mno-relax to assembler when -fno-integrated-as specified

[StephenFan]

Summary

In the past, clang --target=riscv64-unknown-linux-gnu -mno-relax -c hello.s will assemble hello.s without relaxation, but clang --target=riscv64-unknown-linux-gnu -mno-relax -fno-integrated-as -c hello.s doesn't pass the -mno-relax option to assembler, and assemble with relaxation
This patch pass the -mno-relax option to assembler when -fno-integrated-as is specified.

[StephenFan]

fix typo

[jrtc27]

clang/lib/Driver/ToolChains/Gnu.cpp

767

I doubt this does the right thing for -mrelax -mno-relax? I imagine you want Args.hasFlag.

clang/test/Driver/riscv-gnutools.c

19

All of these tests are using -fno-integrated-as, calling it out specifically for this one is misleading

[StephenFan]

Address @jrtc27 's comments

[kito-cheng]

clang/test/Driver/riscv-gnutools.c

19

Yeah, the comment is kind of misleading, actually we didn't check *default* value for -march or -mabi like other two checks(CHECK-RV32IMAC-ILP32/CHECK-RV32IMAFDC-ILP32D).

Maybe like this?

// Check -mno-relax has passed when `-fno-integrated-as` specified

[StephenFan]

Address @kito-cheng 's comments

[StephenFan]

Ping.

[StephenFan]

Ping :)

[asb]

It would be good to have someone else who's reviewed earlier versions of this patch to confirm, but this LGTM. Thanks.

If you were editing it anyway, you could change "has passed" to "is passed" (though it's not a particularly important grammatical fix).

[StephenFan]

Grammatical fix

[StephenFan]

@jrtc27, @kito-cheng Do you have any other comments ? :)

[MaskRay]

clang/lib/Driver/ToolChains/Gnu.cpp

767

Avoid using the error-prone default argument true.

clang/test/Driver/riscv-gnutools.c

44

I don't think repeating this for 64-bit adds value.

It may be more useful to change this to test -mno-relax -mrelax instead.

-target riscv64 should be --target=riscv64-unknown-elf, matching the triple name in %S/Inputs/basic_riscv64_tree .

[StephenFan]

Address @MaskRay 's comments.

1. explicit default-argument to avoid error-prone
2. change riscv64 test to test the case when -mno-relax -mrelax specified.

Is the change of riscv64 test match your @MaskRay requirements?

[asb]

@MaskRay: Are you happy all your comments are addressed?

[MaskRay]

LGTM if you use addOptOutFlag

clang/lib/Driver/ToolChains/Gnu.cpp

767

Args.addOptOutFlag(CmdArgs, options::OPT_mrelax, options::OPT_mno_relax)

[StephenFan]

Thanks @MaskRay, @asb !

clang/lib/Driver/ToolChains/Gnu.cpp

void tools::gnutools::Assembler::ConstructJob(Compilation &C,
   case llvm::Triple::riscv32:
   case llvm::Triple::riscv64: {
     StringRef ABIName = riscv::getRISCVABI(Args, getToolChain().getTriple());
     CmdArgs.push_back("-mabi");
     CmdArgs.push_back(ABIName.data());
     StringRef MArchName = riscv::getRISCVArch(Args, getToolChain().getTriple());
     CmdArgs.push_back("-march");
     CmdArgs.push_back(MArchName.data());
+    if (!Args.hasFlag(options::OPT_mrelax, options::OPT_mno_relax, true))
+      Args.addOptOutFlag(CmdArgs, options::OPT_mrelax, options::OPT_mno_relax);
     break;
   }
   case llvm::Triple::sparc:
   case llvm::Triple::sparcel: {
     CmdArgs.push_back("-32");
     std::string CPU = getCPUName(D, Args, getToolChain().getTriple());
     CmdArgs.push_back(
         sparc::getSparcAsmModeForCPU(CPU, getToolChain().getTriple()));

clang/test/Driver/riscv-gnutools.c

 // Check default on riscv32-unknown-linux-gnu
 // RUN: %clang --target=riscv32-unknown-linux-gnu --gcc-toolchain=%S/Inputs/basic_riscv32_tree -fno-integrated-as %s -### -c \
 // RUN: 2>&1 | FileCheck -check-prefix=CHECK-RV32IMAFDC-ILP32D %s
 
 // Check default when -march=rv32g specified
 // RUN: %clang --target=riscv32-unknown-elf --gcc-toolchain=%S/Inputs/basic_riscv32_tree -fno-integrated-as %s -### -c -march=rv32g \
 // RUN: 2>&1 | FileCheck -check-prefix=CHECK-RV32G-ILP32D %s
 
+// Check -mno-relax is passed when -fno-integrated-as specified
+// RUN: %clang -target riscv32-unknown-elf --gcc-toolchain=%S/Inputs/basic_riscv32_tree -mno-relax -fno-integrated-as %s -### -c \
+// RUN: 2>&1 | FileCheck -check-prefix=CHECK-RV32-NO-RELAX %s
+
 // CHECK-RV32IMAC-ILP32: "{{.*}}as{{(.exe)?}}" "-mabi" "ilp32" "-march" "rv32imac"
 // CHECK-RV32IMAFDC-ILP32D: "{{.*}}as{{(.exe)?}}" "-mabi" "ilp32d" "-march" "rv32imafdc"
 // CHECK-RV32G-ILP32D: "{{.*}}as{{(.exe)?}}" "-mabi" "ilp32d" "-march" "rv32g"
+// CHECK-RV32-NO-RELAX: "{{.*}}as{{(.exe)?}}" "{{.*}}" "-mno-relax"
 
 
 // 64-bit checks
 
 // Check default on riscv64-unknown-elf
 // RUN: %clang --target=riscv64-unknown-elf --gcc-toolchain=%S/Inputs/basic_riscv64_tree -fno-integrated-as %s -### -c \
 // RUN: 2>&1 | FileCheck -check-prefix=CHECK-RV64IMAC-LP64 %s
 
 // Check default on riscv64-unknown-linux-gnu
 // RUN: %clang --target=riscv64-unknown-linux-gnu --gcc-toolchain=%S/Inputs/basic_riscv64_tree -fno-integrated-as %s -### -c \
 // RUN: 2>&1 | FileCheck -check-prefix=CHECK-RV64IMAFDC-LP64D %s
 
 // Check default when -march=rv64g specified
 // RUN: %clang --target=riscv64-unknown-elf --gcc-toolchain=%S/Inputs/basic_riscv64_tree -fno-integrated-as %s -### -c -march=rv64g \
 // RUN: 2>&1 | FileCheck -check-prefix=CHECK-RV64G-LP64D %s
 
+// Check -mno-relax is not passed when -fno-integrated-as specified
+// RUN: %clang -target riscv64-unknown-elf --gcc-toolchain=%S/Inputs/basic_riscv64_tree -mno-relax -mrelax -fno-integrated-as %s -### -c \
+// RUN: 2>&1 | FileCheck -check-prefix=CHECK-RV64-RELAX %s
+
 // CHECK-RV64IMAC-LP64: "{{.*}}as{{(.exe)?}}" "-mabi" "lp64" "-march" "rv64imac"
 // CHECK-RV64IMAFDC-LP64D: "{{.*}}as{{(.exe)?}}" "-mabi" "lp64d" "-march" "rv64imafdc"
 // CHECK-RV64G-LP64D: "{{.*}}as{{(.exe)?}}" "-mabi" "lp64d" "-march" "rv64g"
+// CHECK-RV64-RELAX: "{{.*}}as{{(.exe)?}}"
+// CHECK-RV64-RELAX-NOT: "-mno-relax"