[AVR] Add -mmcu option to the driver

[Lekensteyn]

Summary

Add the AVR-specific -mmcu option for compatibility with GCC (GCC does not use
-mcpu nor -march for AVR). This option is needed to inform the frontend to
define some macros (for example) and the inform the assembler of the allowed
features, so add a test to check that.

Fixes PR#31569

[jroelofs]

include/clang/Driver/Options.td

1613 ↗

(On Diff #87992)

Would it make sense to have mcu be an alias for mcpu instead?

[Lekensteyn]

include/clang/Driver/Options.td

1613 ↗

(On Diff #87992)

That would deviate from the GCC interface, so I have chosen for the current situation:

$ avr-gcc -mmcu=avr2 -o /dev/null x.c
$ avr-gcc -mcpu=avr2 -o /dev/null x.c
avr-gcc: error: unrecognized command line option '-mcpu=avr2'
$ avr-gcc -march=avr2 -o /dev/null x.c
avr-gcc: error: unrecognized command line option '-march=avr2'
$ avr-gcc -v
...
gcc version 6.3.0 (GCC)

[dylanmckay]

include/clang/Driver/Options.td

1613 ↗

(On Diff #87992)

I think @jroelofs means that it is possible to make mmcu an alias of mmcu internally. This would mean we wouldn't need to add AVR-specific getCPUName handling.

[Lekensteyn]

include/clang/Driver/Options.td

1613 ↗

(On Diff #87992)

If mmcu is made an alias of mcpu, wouldn't that mean that both -mcpu and -mmcu would be accepted by driver (undesirable)?
As far as I can see, -target-cpu must be passed to the frontend and assembler, -mcpu= is not recognized as option. And ensuring that getCPUName returns a non-empty string ensures that -target-cpu is passed.

I am quite new to the internals, so please let me know if I misunderstood something :-)

[xiangzhai]

ping :)

[jroelofs]

LGTM

include/clang/Driver/Options.td

1613 ↗

(On Diff #87992)

-mmcu= is the right thing to do, without the alias (I was wrong).

[xiangzhai]

Hi Peter,

Please rebase your patch!

David has already changed the structure of lib/Driver/Tools.cpp in D30372
so it is better to add case llvm::Triple::avr to lib/Driver/ToolChains/CommonArgs.cpp? please point out my fault, thanks!

Regards,
Leslie Zhai

[Lekensteyn]

v2: rebase on commit ad25f8b712f1ef99020fcb7b5e31dd95b39c6112 (trunk@300661)

Based on the context, the change from lib/Driver/Tools.cpp -> lib/Driver/ToolChains/CommonArgs.cpp seems most appropriate (Gnu.cpp seems to be about mapping LLVM options to GCC?)

And please do commit on my behalf, I haven't requested an account yet. Thanks!

[xiangzhai]

Hi Peter,

Thanks for your rebase! LGTM!

Based on the context, the change from lib/Driver/Tools.cpp -> lib/Driver/ToolChains/CommonArgs.cpp seems most appropriate (Gnu.cpp seems to be about mapping LLVM options to GCC?)

Sorry for my mistake! yes, I am maintaining similar patch for GCC toolchain triples :) also rebased to lib/Driver/ToolChains/CommonArgs.cpp for packaging llvm-4.0.0.

And please do commit on my behalf, I haven't requested an account yet. Thanks!

I will commit it for you tomorrow if no Need Review!

Regards,
Leslie Zhai

cfe/trunk/include/clang/Driver/Options.td

 def mno_default_build_attributes : Joined<["-"], "mno-default-build-attributes">, Group<m_Group>;
 def mconstant_cfstrings : Flag<["-"], "mconstant-cfstrings">, Group<clang_ignored_m_Group>;
 def mconsole : Joined<["-"], "mconsole">, Group<m_Group>, Flags<[DriverOption]>;
 def mwindows : Joined<["-"], "mwindows">, Group<m_Group>, Flags<[DriverOption]>;
 def mdll : Joined<["-"], "mdll">, Group<m_Group>, Flags<[DriverOption]>;
 def municode : Joined<["-"], "municode">, Group<m_Group>, Flags<[DriverOption]>;
 def mthreads : Joined<["-"], "mthreads">, Group<m_Group>, Flags<[DriverOption]>;
 def mcpu_EQ : Joined<["-"], "mcpu=">, Group<m_Group>;
+def mmcu_EQ : Joined<["-"], "mmcu=">, Group<m_Group>;
 def mdynamic_no_pic : Joined<["-"], "mdynamic-no-pic">, Group<m_Group>;
 def mfix_and_continue : Flag<["-"], "mfix-and-continue">, Group<clang_ignored_m_Group>;
 def mieee_fp : Flag<["-"], "mieee-fp">, Group<clang_ignored_m_Group>;
 def minline_all_stringops : Flag<["-"], "minline-all-stringops">, Group<clang_ignored_m_Group>;
 def mno_inline_all_stringops : Flag<["-"], "mno-inline-all-stringops">, Group<clang_ignored_m_Group>;
 def malign_double : Flag<["-"], "malign-double">, Group<m_Group>, Flags<[CC1Option]>,
   HelpText<"Align doubles to two words in structs (x86 only)">;
 def mfloat_abi_EQ : Joined<["-"], "mfloat-abi=">, Group<m_Group>;

cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp

std::string tools::getCPUName(const ArgList &Args, const llvm::Triple &T,
   case llvm::Triple::arm:
   case llvm::Triple::armeb:
   case llvm::Triple::thumb:
   case llvm::Triple::thumbeb: {
     StringRef MArch, MCPU;
     arm::getARMArchCPUFromArgs(Args, MArch, MCPU, FromAs);
     return arm::getARMTargetCPU(MCPU, MArch, T);
   }
+
+  case llvm::Triple::avr:
+    if (const Arg *A = Args.getLastArg(options::OPT_mmcu_EQ))
+      return A->getValue();
+    return "";
+
   case llvm::Triple::mips:
   case llvm::Triple::mipsel:
   case llvm::Triple::mips64:
   case llvm::Triple::mips64el: {
     StringRef CPUName;
     StringRef ABIName;
     mips::getMipsCPUAndABI(Args, T, CPUName, ABIName);
     return CPUName;

cfe/trunk/test/Driver/avr-mmcu.c

+// A test for the propagation of the -mmcu option to -cc1 and -cc1as
+
+// RUN: %clang -### -target avr -mmcu=atmega328p -save-temps %s 2>&1 | FileCheck %s
+// CHECK: clang{{.*}} "-cc1" {{.*}} "-target-cpu" "atmega328p"
+// CHECK: clang{{.*}} "-cc1as" {{.*}} "-target-cpu" "atmega328p"