Re: [PATCH] D15899: Add -fno-movt frontend option, to disable movt/movw on ARM

[Eric Christopher via cfe-commits]

echristo accepted this revision.
echristo added a comment.

Sure.

-eric


http://reviews.llvm.org/D15899



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

Re: [PATCH] D15899: Add -fno-movt frontend option, to disable movt/movw on ARM

[Davide Italiano via cfe-commits]

davide added a comment.

In http://reviews.llvm.org/D15899#319918, @dim wrote:

> In http://reviews.llvm.org/D15899#319902, @davide wrote:
>
> > I'm not opposed to this but ... this will likely it only 11 (and maybe 10.3 
> > if backported) -- what about people running <= 10.2 ? Isn't this a problem 
> > for them?
>
>
> People running ports-provided or hand-built clang 3.8.0 or higher on older 
> FreeBSD's can use `-fno-movt`.  For clang versions 3.7.x and earlier, they 
> can still use the old style `-mllvm -arm-use-movt=0`.
>
> We'll fix the FreeBSD build system so it uses the right options for older and 
> newer clang versions.


Sounds like a plan.


http://reviews.llvm.org/D15899



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

[PATCH] D15899: Add -fno-movt frontend option, to disable movt/movw on ARM

[Dimitry Andric via cfe-commits]

dim created this revision.
dim added reviewers: davide, echristo, dexonsmith.
dim added subscribers: emaste, ahatanak, cfe-commits, davide.
Herald added subscribers: rengolin, aemerson.

In rL256641, @davide turned off movt generation by default for FreeBSD.
This was because our ld is very old, and did not support the relocations
for it.  However, Ian Lepore added the support very recently, so we
would like to revert rL256641, and replace it with a new `-fno-movt`
frontend option.  This way, it can be turned off when needed.

http://reviews.llvm.org/D15899

Files:
  include/clang/Driver/Options.td
  lib/Driver/Tools.cpp
  test/Driver/arm-no-movt.c

Index: test/Driver/arm-no-movt.c
===
--- test/Driver/arm-no-movt.c
+++ test/Driver/arm-no-movt.c
@@ -4,11 +4,11 @@
 // RUN: %clang -target armv7-apple-darwin -mkernel -### %s 2>&1 \
 // RUN:| FileCheck %s -check-prefix CHECK-KERNEL
 
-// RUN: %clang -target armv7-gnueabi-freebsd11 -### %s 2>&1 \
-// RUN:| FileCheck %s -check-prefix CHECK-FREEBSD
+// RUN: %clang -target armv7-none-gnueabi -fno-movt -### %s 2>&1 \
+// RUN:| FileCheck %s -check-prefix CHECK-NO-MOVT
 
 // CHECK-DEFAULT-NOT: "-target-feature" "+no-movt"
 
 // CHECK-KERNEL: "-target-feature" "+no-movt"
 
-// CHECK-FREEBSD: "-target-feature" "+no-movt"
+// CHECK-NO-MOVT: "-target-feature" "+no-movt"
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -938,8 +938,8 @@
   if (Args.hasArg(options::OPT_ffixed_r9))
 Features.push_back("+reserve-r9");
 
-  // The kext and FreeBSD linkers don't know how to deal with movw/movt.
-  if (KernelOrKext || Triple.isOSFreeBSD())
+  // The kext linker doesn't know how to deal with movw/movt.
+  if (KernelOrKext || Args.hasArg(options::OPT_fno_movt))
 Features.push_back("+no-movt");
 }
 
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1385,6 +1385,8 @@
 def marm : Flag<["-"], "marm">, Alias;
 def ffixed_r9 : Flag<["-"], "ffixed-r9">, Group,
   HelpText<"Reserve the r9 register (ARM only)">;
+def fno_movt : Flag<["-"], "fno-movt">, Group,
+  HelpText<"Disallow use of movt/movw pairs (ARM only)">;
 def mcrc : Flag<["-"], "mcrc">, Group,
   HelpText<"Allow use of CRC instructions (ARM only)">;
 def mnocrc : Flag<["-"], "mnocrc">, Group,


Index: test/Driver/arm-no-movt.c
===
--- test/Driver/arm-no-movt.c
+++ test/Driver/arm-no-movt.c
@@ -4,11 +4,11 @@
 // RUN: %clang -target armv7-apple-darwin -mkernel -### %s 2>&1 \
 // RUN:| FileCheck %s -check-prefix CHECK-KERNEL
 
-// RUN: %clang -target armv7-gnueabi-freebsd11 -### %s 2>&1 \
-// RUN:| FileCheck %s -check-prefix CHECK-FREEBSD
+// RUN: %clang -target armv7-none-gnueabi -fno-movt -### %s 2>&1 \
+// RUN:| FileCheck %s -check-prefix CHECK-NO-MOVT
 
 // CHECK-DEFAULT-NOT: "-target-feature" "+no-movt"
 
 // CHECK-KERNEL: "-target-feature" "+no-movt"
 
-// CHECK-FREEBSD: "-target-feature" "+no-movt"
+// CHECK-NO-MOVT: "-target-feature" "+no-movt"
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -938,8 +938,8 @@
   if (Args.hasArg(options::OPT_ffixed_r9))
 Features.push_back("+reserve-r9");
 
-  // The kext and FreeBSD linkers don't know how to deal with movw/movt.
-  if (KernelOrKext || Triple.isOSFreeBSD())
+  // The kext linker doesn't know how to deal with movw/movt.
+  if (KernelOrKext || Args.hasArg(options::OPT_fno_movt))
 Features.push_back("+no-movt");
 }
 
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1385,6 +1385,8 @@
 def marm : Flag<["-"], "marm">, Alias;
 def ffixed_r9 : Flag<["-"], "ffixed-r9">, Group,
   HelpText<"Reserve the r9 register (ARM only)">;
+def fno_movt : Flag<["-"], "fno-movt">, Group,
+  HelpText<"Disallow use of movt/movw pairs (ARM only)">;
 def mcrc : Flag<["-"], "mcrc">, Group,
   HelpText<"Allow use of CRC instructions (ARM only)">;
 def mnocrc : Flag<["-"], "mnocrc">, Group,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15899: Add -fno-movt frontend option, to disable movt/movw on ARM

[Davide Italiano via cfe-commits]

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

Hmm, probably they can use the new frontend options so it's fine. LGTM.


http://reviews.llvm.org/D15899



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

Re: [PATCH] D15899: Add -fno-movt frontend option, to disable movt/movw on ARM

[Eric Christopher via cfe-commits]

On Tue, Jan 5, 2016 at 12:29 PM Joerg Sonnenberger via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> On Tue, Jan 05, 2016 at 08:12:34PM +, Dimitry Andric via cfe-commits
> wrote:
> > In rL256641, @davide turned off movt generation by default for FreeBSD.
> > This was because our ld is very old, and did not support the relocations
> > for it.  However, Ian Lepore added the support very recently, so we
> > would like to revert rL256641, and replace it with a new `-fno-movt`
> > frontend option.  This way, it can be turned off when needed.
>
> Is there a precendent for the option name? It should be -m* and not -f*,
> since it is not target independent.
>
>
Yes, agreed, thanks for the catch Joerg.

-eric


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

Re: [PATCH] D15899: Add -fno-movt frontend option, to disable movt/movw on ARM

[Joerg Sonnenberger via cfe-commits]

On Tue, Jan 05, 2016 at 08:12:34PM +, Dimitry Andric via cfe-commits wrote:
> In rL256641, @davide turned off movt generation by default for FreeBSD.
> This was because our ld is very old, and did not support the relocations
> for it.  However, Ian Lepore added the support very recently, so we
> would like to revert rL256641, and replace it with a new `-fno-movt`
> frontend option.  This way, it can be turned off when needed.

Is there a precendent for the option name? It should be -m* and not -f*,
since it is not target independent.

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

Re: [PATCH] D15899: Add -fno-movt frontend option, to disable movt/movw on ARM

[Ed Maste via cfe-commits]

>> Is there a precendent for the option name? It should be -m* and not -f*,
>> since it is not target independent.
>
> Yes, agreed, thanks for the catch Joerg.

Presumably -ffixed_r9 should become -mfixed_r9 as well?
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15899: Add -fno-movt frontend option, to disable movt/movw on ARM

[Dimitry Andric via cfe-commits]

dim updated this revision to Diff 44059.
dim added a comment.

Rename `-fno-movt` to `-mno-movt`.


http://reviews.llvm.org/D15899

Files:
  include/clang/Driver/Options.td
  lib/Driver/Tools.cpp
  test/Driver/arm-no-movt.c

Index: test/Driver/arm-no-movt.c
===
--- test/Driver/arm-no-movt.c
+++ test/Driver/arm-no-movt.c
@@ -4,11 +4,11 @@
 // RUN: %clang -target armv7-apple-darwin -mkernel -### %s 2>&1 \
 // RUN:| FileCheck %s -check-prefix CHECK-KERNEL
 
-// RUN: %clang -target armv7-gnueabi-freebsd11 -### %s 2>&1 \
-// RUN:| FileCheck %s -check-prefix CHECK-FREEBSD
+// RUN: %clang -target armv7-none-gnueabi -mno-movt -### %s 2>&1 \
+// RUN:| FileCheck %s -check-prefix CHECK-NO-MOVT
 
 // CHECK-DEFAULT-NOT: "-target-feature" "+no-movt"
 
 // CHECK-KERNEL: "-target-feature" "+no-movt"
 
-// CHECK-FREEBSD: "-target-feature" "+no-movt"
+// CHECK-NO-MOVT: "-target-feature" "+no-movt"
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -938,8 +938,8 @@
   if (Args.hasArg(options::OPT_ffixed_r9))
 Features.push_back("+reserve-r9");
 
-  // The kext and FreeBSD linkers don't know how to deal with movw/movt.
-  if (KernelOrKext || Triple.isOSFreeBSD())
+  // The kext linker doesn't know how to deal with movw/movt.
+  if (KernelOrKext || Args.hasArg(options::OPT_mno_movt))
 Features.push_back("+no-movt");
 }
 
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1385,6 +1385,8 @@
 def marm : Flag<["-"], "marm">, Alias;
 def ffixed_r9 : Flag<["-"], "ffixed-r9">, Group,
   HelpText<"Reserve the r9 register (ARM only)">;
+def mno_movt : Flag<["-"], "mno-movt">, Group,
+  HelpText<"Disallow use of movt/movw pairs (ARM only)">;
 def mcrc : Flag<["-"], "mcrc">, Group,
   HelpText<"Allow use of CRC instructions (ARM only)">;
 def mnocrc : Flag<["-"], "mnocrc">, Group,


Index: test/Driver/arm-no-movt.c
===
--- test/Driver/arm-no-movt.c
+++ test/Driver/arm-no-movt.c
@@ -4,11 +4,11 @@
 // RUN: %clang -target armv7-apple-darwin -mkernel -### %s 2>&1 \
 // RUN:| FileCheck %s -check-prefix CHECK-KERNEL
 
-// RUN: %clang -target armv7-gnueabi-freebsd11 -### %s 2>&1 \
-// RUN:| FileCheck %s -check-prefix CHECK-FREEBSD
+// RUN: %clang -target armv7-none-gnueabi -mno-movt -### %s 2>&1 \
+// RUN:| FileCheck %s -check-prefix CHECK-NO-MOVT
 
 // CHECK-DEFAULT-NOT: "-target-feature" "+no-movt"
 
 // CHECK-KERNEL: "-target-feature" "+no-movt"
 
-// CHECK-FREEBSD: "-target-feature" "+no-movt"
+// CHECK-NO-MOVT: "-target-feature" "+no-movt"
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -938,8 +938,8 @@
   if (Args.hasArg(options::OPT_ffixed_r9))
 Features.push_back("+reserve-r9");
 
-  // The kext and FreeBSD linkers don't know how to deal with movw/movt.
-  if (KernelOrKext || Triple.isOSFreeBSD())
+  // The kext linker doesn't know how to deal with movw/movt.
+  if (KernelOrKext || Args.hasArg(options::OPT_mno_movt))
 Features.push_back("+no-movt");
 }
 
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1385,6 +1385,8 @@
 def marm : Flag<["-"], "marm">, Alias;
 def ffixed_r9 : Flag<["-"], "ffixed-r9">, Group,
   HelpText<"Reserve the r9 register (ARM only)">;
+def mno_movt : Flag<["-"], "mno-movt">, Group,
+  HelpText<"Disallow use of movt/movw pairs (ARM only)">;
 def mcrc : Flag<["-"], "mcrc">, Group,
   HelpText<"Allow use of CRC instructions (ARM only)">;
 def mnocrc : Flag<["-"], "mnocrc">, Group,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15899: Add -fno-movt frontend option, to disable movt/movw on ARM

[Ed Maste via cfe-commits]

emaste accepted this revision.
emaste added a reviewer: emaste.
emaste added a comment.

This LGTM


http://reviews.llvm.org/D15899



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

Re: [PATCH] D15899: Add -fno-movt frontend option, to disable movt/movw on ARM

[Dimitry Andric via cfe-commits]

dim added a comment.

In http://reviews.llvm.org/D15899#319902, @davide wrote:

> I'm not opposed to this but ... this will likely it only 11 (and maybe 10.3 
> if backported) -- what about people running <= 10.2 ? Isn't this a problem 
> for them?


People running ports-provided or hand-built clang 3.8.0 or higher on older 
FreeBSD's can use `-fno-movt`.  For clang versions 3.7.x and earlier, they can 
still use the old style `-mllvm -arm-use-movt=0`.

We'll fix the FreeBSD build system so it uses the right options for older and 
newer clang versions.


http://reviews.llvm.org/D15899



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

Re: [PATCH] D15899: Add -fno-movt frontend option, to disable movt/movw on ARM

[Davide Italiano via cfe-commits]

davide added a comment.

I'm not opposed to this but ... this will likely it only 11 (and maybe 10.3 if 
backported) -- what about people running <= 10.2 ? Isn't this a problem for 
them?


http://reviews.llvm.org/D15899



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

Re: [PATCH] D15899: Add -fno-movt frontend option, to disable movt/movw on ARM

[Eric Christopher via cfe-commits]

On Tue, Jan 5, 2016 at 12:43 PM Ed Maste  wrote:

> >> Is there a precendent for the option name? It should be -m* and not -f*,
> >> since it is not target independent.
> >
> > Yes, agreed, thanks for the catch Joerg.
>
> Presumably -ffixed_r9 should become -mfixed_r9 as well?
>

That's a bit different in that it matches the -ffixed- command line
option from gcc - just hardcoded to r9.

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