[PATCH] D29827: [AVR] Add -mmcu option to the driver

[Leslie Zhai via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL300818: [AVR] Add -mmcu option to the driver (authored by 
xiangzhai).

Changed prior to commit:
  https://reviews.llvm.org/D29827?vs=95707&id=95879#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D29827

Files:
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
  cfe/trunk/test/Driver/avr-mmcu.c


Index: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
@@ -261,6 +261,12 @@
 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:
Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -1661,6 +1661,7 @@
 def municode : Joined<["-"], "municode">, Group, 
Flags<[DriverOption]>;
 def mthreads : Joined<["-"], "mthreads">, Group, 
Flags<[DriverOption]>;
 def mcpu_EQ : Joined<["-"], "mcpu=">, Group;
+def mmcu_EQ : Joined<["-"], "mmcu=">, Group;
 def mdynamic_no_pic : Joined<["-"], "mdynamic-no-pic">, Group;
 def mfix_and_continue : Flag<["-"], "mfix-and-continue">, 
Group;
 def mieee_fp : Flag<["-"], "mieee-fp">, Group;
Index: cfe/trunk/test/Driver/avr-mmcu.c
===
--- cfe/trunk/test/Driver/avr-mmcu.c
+++ cfe/trunk/test/Driver/avr-mmcu.c
@@ -0,0 +1,5 @@
+// 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"


Index: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
@@ -261,6 +261,12 @@
 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:
Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -1661,6 +1661,7 @@
 def municode : Joined<["-"], "municode">, Group, Flags<[DriverOption]>;
 def mthreads : Joined<["-"], "mthreads">, Group, Flags<[DriverOption]>;
 def mcpu_EQ : Joined<["-"], "mcpu=">, Group;
+def mmcu_EQ : Joined<["-"], "mmcu=">, Group;
 def mdynamic_no_pic : Joined<["-"], "mdynamic-no-pic">, Group;
 def mfix_and_continue : Flag<["-"], "mfix-and-continue">, Group;
 def mieee_fp : Flag<["-"], "mieee-fp">, Group;
Index: cfe/trunk/test/Driver/avr-mmcu.c
===
--- cfe/trunk/test/Driver/avr-mmcu.c
+++ cfe/trunk/test/Driver/avr-mmcu.c
@@ -0,0 +1,5 @@
+// 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"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29827: [AVR] Add -mmcu option to the driver

[Leslie Zhai via Phabricator via cfe-commits]

xiangzhai accepted this revision.
xiangzhai added a comment.
This revision is now accepted and ready to land.

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


https://reviews.llvm.org/D29827



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29827: [AVR] Add -mmcu option to the driver

[Peter Wu via Phabricator via cfe-commits]

Lekensteyn updated this revision to Diff 95707.
Lekensteyn added a comment.

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?)


https://reviews.llvm.org/D29827

Files:
  include/clang/Driver/Options.td
  lib/Driver/ToolChains/CommonArgs.cpp
  test/Driver/avr-mmcu.c


Index: test/Driver/avr-mmcu.c
===
--- /dev/null
+++ test/Driver/avr-mmcu.c
@@ -0,0 +1,5 @@
+// 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"
Index: lib/Driver/ToolChains/CommonArgs.cpp
===
--- lib/Driver/ToolChains/CommonArgs.cpp
+++ lib/Driver/ToolChains/CommonArgs.cpp
@@ -261,6 +261,12 @@
 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:
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1655,6 +1655,7 @@
 def municode : Joined<["-"], "municode">, Group, 
Flags<[DriverOption]>;
 def mthreads : Joined<["-"], "mthreads">, Group, 
Flags<[DriverOption]>;
 def mcpu_EQ : Joined<["-"], "mcpu=">, Group;
+def mmcu_EQ : Joined<["-"], "mmcu=">, Group;
 def mdynamic_no_pic : Joined<["-"], "mdynamic-no-pic">, Group;
 def mfix_and_continue : Flag<["-"], "mfix-and-continue">, 
Group;
 def mieee_fp : Flag<["-"], "mieee-fp">, Group;


Index: test/Driver/avr-mmcu.c
===
--- /dev/null
+++ test/Driver/avr-mmcu.c
@@ -0,0 +1,5 @@
+// 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"
Index: lib/Driver/ToolChains/CommonArgs.cpp
===
--- lib/Driver/ToolChains/CommonArgs.cpp
+++ lib/Driver/ToolChains/CommonArgs.cpp
@@ -261,6 +261,12 @@
 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:
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1655,6 +1655,7 @@
 def municode : Joined<["-"], "municode">, Group, Flags<[DriverOption]>;
 def mthreads : Joined<["-"], "mthreads">, Group, Flags<[DriverOption]>;
 def mcpu_EQ : Joined<["-"], "mcpu=">, Group;
+def mmcu_EQ : Joined<["-"], "mmcu=">, Group;
 def mdynamic_no_pic : Joined<["-"], "mdynamic-no-pic">, Group;
 def mfix_and_continue : Flag<["-"], "mfix-and-continue">, Group;
 def mieee_fp : Flag<["-"], "mieee-fp">, Group;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29827: [AVR] Add -mmcu option to the driver

[Leslie Zhai via Phabricator via cfe-commits]

xiangzhai added a comment.

Hi Jonathan,

Peter might do not have commit access, please commit it on his behalf, thanks a 
lot!

Regards,
Leslie Zhai


https://reviews.llvm.org/D29827



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29827: [AVR] Add -mmcu option to the driver

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs accepted this revision.
jroelofs added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: include/clang/Driver/Options.td:1613
 def mcpu_EQ : Joined<["-"], "mcpu=">, Group;
+def mmcu_EQ : Joined<["-"], "mmcu=">, Group;
 def mdynamic_no_pic : Joined<["-"], "mdynamic-no-pic">, Group;

Lekensteyn wrote:
> dylanmckay wrote:
> > Lekensteyn wrote:
> > > jroelofs wrote:
> > > > Would it make sense to have mcu be an alias for mcpu instead?
> > > 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)
> > > ```
> > 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.
> 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 :-)
-mmcu= is the right thing to do, without the alias (I was wrong).


https://reviews.llvm.org/D29827



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29827: [AVR] Add -mmcu option to the driver

[Leslie Zhai via Phabricator via cfe-commits]

xiangzhai added a comment.

ping :)


https://reviews.llvm.org/D29827



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29827: [AVR] Add -mmcu option to the driver

[Peter Wu via Phabricator via cfe-commits]

Lekensteyn added inline comments.



Comment at: include/clang/Driver/Options.td:1613
 def mcpu_EQ : Joined<["-"], "mcpu=">, Group;
+def mmcu_EQ : Joined<["-"], "mmcu=">, Group;
 def mdynamic_no_pic : Joined<["-"], "mdynamic-no-pic">, Group;

dylanmckay wrote:
> Lekensteyn wrote:
> > jroelofs wrote:
> > > Would it make sense to have mcu be an alias for mcpu instead?
> > 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)
> > ```
> 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.
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 :-)


https://reviews.llvm.org/D29827



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29827: [AVR] Add -mmcu option to the driver

[Dylan McKay via Phabricator via cfe-commits]

dylanmckay added inline comments.



Comment at: include/clang/Driver/Options.td:1613
 def mcpu_EQ : Joined<["-"], "mcpu=">, Group;
+def mmcu_EQ : Joined<["-"], "mmcu=">, Group;
 def mdynamic_no_pic : Joined<["-"], "mdynamic-no-pic">, Group;

Lekensteyn wrote:
> jroelofs wrote:
> > Would it make sense to have mcu be an alias for mcpu instead?
> 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)
> ```
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.


https://reviews.llvm.org/D29827



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29827: [AVR] Add -mmcu option to the driver

[Peter Wu via Phabricator via cfe-commits]

Lekensteyn added inline comments.



Comment at: include/clang/Driver/Options.td:1613
 def mcpu_EQ : Joined<["-"], "mcpu=">, Group;
+def mmcu_EQ : Joined<["-"], "mmcu=">, Group;
 def mdynamic_no_pic : Joined<["-"], "mdynamic-no-pic">, Group;

jroelofs wrote:
> Would it make sense to have mcu be an alias for mcpu instead?
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)
```


https://reviews.llvm.org/D29827



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29827: [AVR] Add -mmcu option to the driver

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added inline comments.



Comment at: include/clang/Driver/Options.td:1613
 def mcpu_EQ : Joined<["-"], "mcpu=">, Group;
+def mmcu_EQ : Joined<["-"], "mmcu=">, Group;
 def mdynamic_no_pic : Joined<["-"], "mdynamic-no-pic">, Group;

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


https://reviews.llvm.org/D29827



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29827: [AVR] Add -mmcu option to the driver

[Peter Wu via Phabricator via cfe-commits]

Lekensteyn created this revision.

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


https://reviews.llvm.org/D29827

Files:
  include/clang/Driver/Options.td
  lib/Driver/Tools.cpp
  test/Driver/avr-mmcu.c


Index: test/Driver/avr-mmcu.c
===
--- /dev/null
+++ test/Driver/avr-mmcu.c
@@ -0,0 +1,5 @@
+// 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"
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -2136,6 +2136,12 @@
 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:
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1610,6 +1610,7 @@
 def municode : Joined<["-"], "municode">, Group, 
Flags<[DriverOption]>;
 def mthreads : Joined<["-"], "mthreads">, Group, 
Flags<[DriverOption]>;
 def mcpu_EQ : Joined<["-"], "mcpu=">, Group;
+def mmcu_EQ : Joined<["-"], "mmcu=">, Group;
 def mdynamic_no_pic : Joined<["-"], "mdynamic-no-pic">, Group;
 def mfix_and_continue : Flag<["-"], "mfix-and-continue">, 
Group;
 def mieee_fp : Flag<["-"], "mieee-fp">, Group;


Index: test/Driver/avr-mmcu.c
===
--- /dev/null
+++ test/Driver/avr-mmcu.c
@@ -0,0 +1,5 @@
+// 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"
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -2136,6 +2136,12 @@
 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:
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1610,6 +1610,7 @@
 def municode : Joined<["-"], "municode">, Group, Flags<[DriverOption]>;
 def mthreads : Joined<["-"], "mthreads">, Group, Flags<[DriverOption]>;
 def mcpu_EQ : Joined<["-"], "mcpu=">, Group;
+def mmcu_EQ : Joined<["-"], "mmcu=">, Group;
 def mdynamic_no_pic : Joined<["-"], "mdynamic-no-pic">, Group;
 def mfix_and_continue : Flag<["-"], "mfix-and-continue">, Group;
 def mieee_fp : Flag<["-"], "mieee-fp">, Group;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits