[Driver] Don't forward -m[no-]unaligned-access options to GCC when assembling/linking

[mcrosier]

Summary

I'm not particularly fond of the fix, but at the same time I'm finding it difficult to come up with something better.

Chad

[echristo]

Should we instead be translating and passing the option expected? At any rate, it looks like we're stripping out in ConstructJob rather than forwardToGCC typically.

lib/Driver/ToolChains/Gnu.cpp

46

effect->affect

[• espindola]

Can we instead drop support for using gcc as an assembler?

[echristo]

In D45092#1055090, @espindola wrote:

Can we instead drop support for using gcc as an assembler?

I'd prefer not at the moment, being able to use an external assembler could be useful? Maybe having support as a "-fuse-as" with no argument cleaning would be something we could do?

[• espindola]

In D45092#1055124, @echristo wrote:

In D45092#1055090, @espindola wrote:

Can we instead drop support for using gcc as an assembler?

I'd prefer not at the moment, being able to use an external assembler could be useful? Maybe having support as a "-fuse-as" with no argument cleaning would be something we could do?

We could add a -fuse-as as a convenience, but similar to -fuse-ld it should call as directly, not via gcc.

It should currently be possible to select the assembler with -B too.

[echristo]

In D45092#1055127, @espindola wrote:

In D45092#1055124, @echristo wrote:

In D45092#1055090, @espindola wrote:

Can we instead drop support for using gcc as an assembler?

I'd prefer not at the moment, being able to use an external assembler could be useful? Maybe having support as a "-fuse-as" with no argument cleaning would be something we could do?

We could add a -fuse-as as a convenience, but similar to -fuse-ld it should call as directly, not via gcc.

It should currently be possible to select the assembler with -B too.

Ah well, I just find it nicer to use gcc as an interface if we're doing that. No one should really use it otherwise.

[mcrosier]

Update based on Eric's feedback.

[mcrosier]

In D45092#1053793, @echristo wrote:

Should we instead be translating and passing the option expected?

My understanding is that these are compile-time only flags, so I'm not sure there's much value in translating them and then passing them to GCC unless I've missed something.

[echristo]

In D45092#1063584, @mcrosier wrote:

In D45092#1053793, @echristo wrote:

Should we instead be translating and passing the option expected?

My understanding is that these are compile-time only flags, so I'm not sure there's much value in translating them and then passing them to GCC unless I've missed something.

Good point :)

-eric

[mcrosier]

Thanks, Eric!

lib/Driver/ToolChains/Gnu.cpp

 using namespace llvm::opt;
 
 void tools::GnuTool::anchor() {}
 
 static bool forwardToGCC(const Option &O) {
   // Don't forward inputs from the original command line.  They are added from
   // InputInfoList.
   return O.getKind() != Option::InputClass &&
-         !O.hasFlag(options::DriverOption) && !O.hasFlag(options::LinkerInput);
+         !O.hasFlag(options::DriverOption) &&
+         !O.hasFlag(options::LinkerInput) &&
+         // Filter out -mno-unaligned-access since GCC expects -mstrict-align
+         // instead for some targets and this flag shouldn't effect assembly
+         // or link steps.
+         O.getID() != options::OPT_munaligned_access &&
+         O.getID() != options::OPT_mno_unaligned_access;
 }
 
 // Switch CPU names not recognized by GNU assembler to a close CPU that it does
 // recognize, instead of a lower march from being picked in the absence of a cpu
 // flag.
 static void normalizeCPUNamesForAssembler(const ArgList &Args,
                                           ArgStringList &CmdArgs) {
   if (Arg *A = Args.getLastArg(options::OPT_mcpu_EQ)) {

test/Driver/gcc_forward.c

 // CHECK-NOT: "-Wdocumentation"
 // CHECK: "-o" "a.out"
 
 // Check that we're not forwarding -g options to the assembler
 // RUN: %clang -g -target x86_64-unknown-linux-gnu -no-integrated-as -c %s -### 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-ASM %s
 // CHECK-ASM: as
 // CHECK-ASM-NOT: "-g"
+
+// Check that we're not forwarding -mno-unaligned-access since GCC expects
+// -mstrict-align instead for some targets and this flag shouldn't effect
+// linking.
+// RUN: %clang -target aarch64-none-elf -mno-unaligned-access %s -### 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-ARM %s
+// CHECK-ARM: gcc{{[^"]*}}"
+// CHECK-ARM-NOT: -mno-unaligned-access