[clang][driver] Support option '-mcpu' on target AVR.

[benshi001]

Summary

The options '-mcpu' and '-mmcu' has the same functionality except
'-mmcu' is used only for target AVR and MSP430. I think newer AVR
projects should use clang common style options instead of older
avr-gcc style.

Currently we keep '-mmcu' for compatibility with avr-gcc,
and '-mcpu' is superior to '-mmcu'.

[benshi001]

clang/test/Driver/avr-mmcu.c

48

These 4 tests are redundant with their previous lines, so they should be removed.

[arichardson]

clang/lib/Driver/ToolChains/CommonArgs.cpp

391

It looks to me that -mcpu will always take precedence over -mmcu with this code even if mmcu comes later in the command line. Wouldn't it make more sense to treat them as aliases?

[benshi001]

clang/lib/Driver/ToolChains/CommonArgs.cpp

391

That's really a good idea. Thanks!

[MaskRay]

This makes -mmcu= available for non-AVR targets, which is not right.
Even for AVR, if this is a plain alias, is this really necessary? Usually having more than one aliases for one functionality just create confusion as users don't know which one is the canonical.
I think one choice is for AVR users to migrate -mmcu= (if any) to -mcpu=, if there are a significant number of uses. Making a project buildable with Clang usually requires porting efforts, and this request may not be unreasonable.

[benshi001]

In D139305#3974946, @MaskRay wrote:

This makes -mmcu= available for non-AVR targets, which is not right.
Even for AVR, if this is a plain alias, is this really necessary? Usually having more than one aliases for one functionality just create confusion as users don't know which one is the canonical.
I think one choice is for AVR users to migrate -mmcu= (if any) to -mcpu=, if there are a significant number of uses. Making a project buildable with Clang usually requires porting efforts, and this request may not be unreasonable.

I have changed to

1. Both '-mcpu' and '-mmcu' are supported for AVR, and '-mmcu' is not available to others;
2. '-mcpu' is superior to '-mmcu', and '-mcpu' is encouraged for newer clang built AVR projects;
3. A warning will emit if both are specified.

[MaskRay]

I think newr AVR projects should use clang common style options instead of older avr-gcc style.

Does GCC prefer -mcpu= as well? If not, raise a PR there? This seems like a decision we should not unilaterally make on llvm-project side: it's certainly something GCC cares about as well.

[aykevl]

I think having -mcpu as an alias for -mmcu on AVR is fine, but I don't think it's very useful to be honest. The -mmcu (or -mcpu) flag is inherently specific to that particular chip anyway so compatibility is not an issue. In fact, having two flags may even introduce noise and confusion.
I wouldn't be opposed to it though if there is demand for it.

In D139305#3987969, @MaskRay wrote:

Does GCC prefer -mcpu= as well? If not, raise a PR there? This seems like a decision we should not unilaterally make on llvm-project side: it's certainly something GCC cares about as well.

I guess, but avr-gcc is already barely maintained. Most people use an older version (like 5.2.0) because the newer versions produce absolutely terrible code. So I don't think it matters much.

[benshi001]

In D139305#4013217, @aykevl wrote:

I think having -mcpu as an alias for -mmcu on AVR is fine, but I don't think it's very useful to be honest. The -mmcu (or -mcpu) flag is inherently specific to that particular chip anyway so compatibility is not an issue. In fact, having two flags may even introduce noise and confusion.
I wouldn't be opposed to it though if there is demand for it.

In D139305#3987969, @MaskRay wrote:

Does GCC prefer -mcpu= as well? If not, raise a PR there? This seems like a decision we should not unilaterally make on llvm-project side: it's certainly something GCC cares about as well.

I guess, but avr-gcc is already barely maintained. Most people use an older version (like 5.2.0) because the newer versions produce absolutely terrible code. So I don't think it matters much.

I see. Thanks.

clang/lib/Driver/ToolChains/CommonArgs.cpp

std::string tools::getCPUName(const Driver &D, const ArgList &Args,
   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:
+    // We encourage users to use '-mcpu' as their first choice,
+    // rather than the old '-mmcu' (inherited from avr-gcc).
+    if (const Arg *A = Args.getLastArg(options::OPT_mcpu_EQ))
+      return A->getValue();
     if (const Arg *A = Args.getLastArg(options::OPT_mmcu_EQ))
       return A->getValue();
     return "";
 
   case llvm::Triple::m68k:
     return m68k::getM68kTargetCPU(Args);
 
   case llvm::Triple::mips:
   case llvm::Triple::mipsel:
   case llvm::Triple::mips64:
   case llvm::Triple::mips64el: {

clang/test/Driver/avr-mmcu.c

 // A test for the propagation of the -mmcu option to -cc1 and -cc1as
 
+// RUN: %clang -### --target=avr -mcpu=attiny11 -save-temps %s 2>&1 | FileCheck -check-prefix=CHECK0 %s
 // RUN: %clang -### --target=avr -mmcu=attiny11 -save-temps %s 2>&1 | FileCheck -check-prefix=CHECK0 %s
 // CHECK0: "-cc1" {{.*}} "-target-cpu" "attiny11"
 // CHECK0: "-cc1as" {{.*}} "-target-cpu" "attiny11"
 
+// RUN: %clang -### --target=avr -mcpu=at90s2313 -save-temps %s 2>&1 | FileCheck -check-prefix=CHECK1 %s
 // RUN: %clang -### --target=avr -mmcu=at90s2313 -save-temps %s 2>&1 | FileCheck -check-prefix=CHECK1 %s
 // CHECK1: "-cc1" {{.*}} "-target-cpu" "at90s2313"
 // CHECK1: "-cc1as" {{.*}} "-target-cpu" "at90s2313"
 
+// RUN: %clang -### --target=avr -mcpu=at90s8515 -save-temps %s 2>&1 | FileCheck -check-prefix=CHECK2 %s
 // RUN: %clang -### --target=avr -mmcu=at90s8515 -save-temps %s 2>&1 | FileCheck -check-prefix=CHECK2 %s
 // CHECK2: "-cc1" {{.*}} "-target-cpu" "at90s8515"
 // CHECK2: "-cc1as" {{.*}} "-target-cpu" "at90s8515"
 
 // RUN: %clang -### --target=avr -mmcu=attiny13a -save-temps %s 2>&1 | FileCheck -check-prefix=CHECK3 %s
 // CHECK3: "-cc1" {{.*}} "-target-cpu" "attiny13a"
 // CHECK3: "-cc1as" {{.*}} "-target-cpu" "attiny13a"
 
 // RUN: %clang -### --target=avr -mmcu=attiny88 -save-temps %s 2>&1 | FileCheck -check-prefix=CHECK4 %s
 // CHECK4: "-cc1" {{.*}} "-target-cpu" "attiny88"
 // CHECK4: "-cc1as" {{.*}} "-target-cpu" "attiny88"
 
-// RUN: %clang -### --target=avr -mmcu=attiny88 -save-temps %s 2>&1 | FileCheck -check-prefix=CHECK5 %s
-// CHECK5: "-cc1" {{.*}} "-target-cpu" "attiny88"
-// CHECK5: "-cc1as" {{.*}} "-target-cpu" "attiny88"
-
 // RUN: %clang -### --target=avr -mmcu=atmega8u2 -save-temps %s 2>&1 | FileCheck -check-prefix=CHECK6 %s
 // CHECK6: "-cc1" {{.*}} "-target-cpu" "atmega8u2"
 // CHECK6: "-cc1as" {{.*}} "-target-cpu" "atmega8u2"
 
-// RUN: %clang -### --target=avr -mmcu=atmega8u2 -save-temps %s 2>&1 | FileCheck -check-prefix=CHECK7 %s
-// CHECK7: "-cc1" {{.*}} "-target-cpu" "atmega8u2"
-// CHECK7: "-cc1as" {{.*}} "-target-cpu" "atmega8u2"
-
 // RUN: %clang -### --target=avr -mmcu=atmega8a -save-temps %s 2>&1 | FileCheck -check-prefix=CHECK8 %s
 // CHECK8: "-cc1" {{.*}} "-target-cpu" "atmega8a"
 // CHECK8: "-cc1as" {{.*}} "-target-cpu" "atmega8a"
 
-// RUN: %clang -### --target=avr -mmcu=atmega8a -save-temps %s 2>&1 | FileCheck -check-prefix=CHECK9 %s
-// CHECK9: "-cc1" {{.*}} "-target-cpu" "atmega8a"
-// CHECK9: "-cc1as" {{.*}} "-target-cpu" "atmega8a"
-
 // RUN: %clang -### --target=avr -mmcu=atmega16a -save-temps %s 2>&1 | FileCheck -check-prefix=CHECKa %s
 // CHECKa: "-cc1" {{.*}} "-target-cpu" "atmega16a"
 // CHECKa: "-cc1as" {{.*}} "-target-cpu" "atmega16a"
 
-// RUN: %clang -### --target=avr -mmcu=atmega16a -save-temps %s 2>&1 | FileCheck -check-prefix=CHECKb %s
-// CHECKb: "-cc1" {{.*}} "-target-cpu" "atmega16a"
-// CHECKb: "-cc1as" {{.*}} "-target-cpu" "atmega16a"
-
 // RUN: %clang -### --target=avr -mmcu=atmega128a -save-temps %s 2>&1 | FileCheck -check-prefix=CHECKc %s
 // CHECKc: "-cc1" {{.*}} "-target-cpu" "atmega128a"
 // CHECKc: "-cc1as" {{.*}} "-target-cpu" "atmega128a"
 
 // RUN: %clang -### --target=avr -mmcu=atmega2560 -save-temps %s 2>&1 | FileCheck -check-prefix=CHECKd %s
 // CHECKd: "-cc1" {{.*}} "-target-cpu" "atmega2560"
 // CHECKd: "-cc1as" {{.*}} "-target-cpu" "atmega2560"
 
 // RUN: %clang -### --target=avr -mmcu=attiny10 -save-temps %s 2>&1 | FileCheck -check-prefix=CHECKe %s
 // CHECKe: "-cc1" {{.*}} "-target-cpu" "attiny10"
 // CHECKe: "-cc1as" {{.*}} "-target-cpu" "attiny10"
 
 // RUN: %clang -### --target=avr -mmcu=atxmega16a4 -save-temps %s 2>&1 | FileCheck -check-prefix=CHECKf %s
 // CHECKf: "-cc1" {{.*}} "-target-cpu" "atxmega16a4"
 // CHECKf: "-cc1as" {{.*}} "-target-cpu" "atxmega16a4"
 
 // RUN: %clang -### --target=avr -mmcu=atxmega64b1 -save-temps %s 2>&1 | FileCheck -check-prefix=CHECKg %s
 // CHECKg: "-cc1" {{.*}} "-target-cpu" "atxmega64b1"
 // CHECKg: "-cc1as" {{.*}} "-target-cpu" "atxmega64b1"
 
 // RUN: %clang -### --target=avr -mmcu=atxmega64a1u -save-temps %s 2>&1 | FileCheck -check-prefix=CHECKh %s
 // CHECKh: "-cc1" {{.*}} "-target-cpu" "atxmega64a1u"
 // CHECKh: "-cc1as" {{.*}} "-target-cpu" "atxmega64a1u"
 
+// RUN: %clang -### --target=avr -mcpu=atxmega128a3u -save-temps %s 2>&1 | FileCheck -check-prefix=CHECKj %s
 // RUN: %clang -### --target=avr -mmcu=atxmega128a3u -save-temps %s 2>&1 | FileCheck -check-prefix=CHECKj %s
 // CHECKj: "-cc1" {{.*}} "-target-cpu" "atxmega128a3u"
 // CHECKj: "-cc1as" {{.*}} "-target-cpu" "atxmega128a3u"
 
+// RUN: %clang -### --target=avr -mcpu=atxmega128a4u -save-temps %s 2>&1 | FileCheck -check-prefix=CHECKi %s
 // RUN: %clang -### --target=avr -mmcu=atxmega128a4u -save-temps %s 2>&1 | FileCheck -check-prefix=CHECKi %s
 // CHECKi: "-cc1" {{.*}} "-target-cpu" "atxmega128a4u"
 // CHECKi: "-cc1as" {{.*}} "-target-cpu" "atxmega128a4u"
+
+// RUN: %clang --target=avr -mcpu=atmega328 -mmcu=atmega168 -c %s 2>&1 | FileCheck -check-prefix=CHECKz %s
+// CHECKz: warning: argument unused during compilation: '-mmcu=atmega168'