diff mbox

[rs6000] Fix PR target/68872, make -mcpu=powerpc64le pass correct assembler option

Message ID 1450804433.3482.17.camel@vnet.ibm.com
State New
Headers show

Commit Message

Peter Bergner Dec. 22, 2015, 5:13 p.m. UTC
Currently, -mcpu=powerpc64le correctly sets the TARGET_* flags for an
LE compile, meaning it mimics a -mcpu=power8 compile, but it doesn't
pass the correct -mpower8/-mpwr8 option to the assembler, so we die
with lots of assembler errors on POWER8 instructions.  This patch
fixes things so we pass the correct assembler option when using
-mcpu=powerpc64le.

This passes bootstrap/regtesting on powerpc64le-linux.  Ok for mainline?
Ok for the release branches too after testing?

Peter


gcc/
	PR target/68772
	* config/rs6000/rs6000.h (ASM_CPU_SPEC): For -mcpu=powerpc64le,
	pass %(asm_cpu_power8)/-mpwr8.
	* config/rs6000/aix53.h: Likewise.
	* config/rs6000/aix61.h: Likewise.
	* config/rs6000/aix71.h: Likewise.

gcc/testsuite/
        PR target/68772
        * gcc.target/powerpc/pr68872.c: New test.

Comments

David Edelsohn Dec. 22, 2015, 6:36 p.m. UTC | #1
On Tue, Dec 22, 2015 at 12:13 PM, Peter Bergner <bergner@vnet.ibm.com> wrote:
> Currently, -mcpu=powerpc64le correctly sets the TARGET_* flags for an
> LE compile, meaning it mimics a -mcpu=power8 compile, but it doesn't
> pass the correct -mpower8/-mpwr8 option to the assembler, so we die
> with lots of assembler errors on POWER8 instructions.  This patch
> fixes things so we pass the correct assembler option when using
> -mcpu=powerpc64le.
>
> This passes bootstrap/regtesting on powerpc64le-linux.  Ok for mainline?
> Ok for the release branches too after testing?
>
> Peter
>
>
> gcc/
>         PR target/68772
>         * config/rs6000/rs6000.h (ASM_CPU_SPEC): For -mcpu=powerpc64le,
>         pass %(asm_cpu_power8)/-mpwr8.
>         * config/rs6000/aix53.h: Likewise.
>         * config/rs6000/aix61.h: Likewise.
>         * config/rs6000/aix71.h: Likewise.
>
> gcc/testsuite/
>         PR target/68772
>         * gcc.target/powerpc/pr68872.c: New test.

AIX does not support PPC64LE, so the aix53.h, aix61.h and aix71.h
changes should not be applied.

Ultimately rs6000_file_start ".machine" directive support should be
strengthened to handle -mcpu=.

Okay with just the rs6000.h change and new testcase.

Thanks, David
Peter Bergner Dec. 22, 2015, 7:30 p.m. UTC | #2
On Tue, 2015-12-22 at 13:36 -0500, David Edelsohn wrote:
> On Tue, Dec 22, 2015 at 12:13 PM, Peter Bergner <bergner@vnet.ibm.com> wrote:
> AIX does not support PPC64LE, so the aix53.h, aix61.h and aix71.h
> changes should not be applied.

Heh, I should know better.  Thanks for catching that.


> Ultimately rs6000_file_start ".machine" directive support should be
> strengthened to handle -mcpu=.

I'm not sure there is anything to do there.  The code that emits the
.machine assembler directive uses rs6000_isa_flags which should be
set correctly, regardless of whether we use -mcpu= with power8 or
powerpc64le.  ...although, the code as is never outputs a .machine
on LE, since the code that guards the emitting of .machine doesn't
ever seem to be true on LE.  At least I can't seem to get a .machine
generated with my LE build.


> Okay with just the rs6000.h change and new testcase.

Ok, committed to trunk as revision 231905.  Thanks.

Peter
David Edelsohn Dec. 22, 2015, 7:33 p.m. UTC | #3
On Tue, Dec 22, 2015 at 2:30 PM, Peter Bergner <bergner@vnet.ibm.com> wrote:
> On Tue, 2015-12-22 at 13:36 -0500, David Edelsohn wrote:
>> On Tue, Dec 22, 2015 at 12:13 PM, Peter Bergner <bergner@vnet.ibm.com> wrote:

>> Ultimately rs6000_file_start ".machine" directive support should be
>> strengthened to handle -mcpu=.
>
> I'm not sure there is anything to do there.  The code that emits the
> .machine assembler directive uses rs6000_isa_flags which should be
> set correctly, regardless of whether we use -mcpu= with power8 or
> powerpc64le.  ...although, the code as is never outputs a .machine
> on LE, since the code that guards the emitting of .machine doesn't
> ever seem to be true on LE.  At least I can't seem to get a .machine
> generated with my LE build.

Currently, the .machine directive only is emitted if -mcpu= is not
used.  We should avoid -mcpu= setting assembler command line options
and instead switch to use .machine.  The .machine path needs to be
tested more thoroughly in the presence of -mcpu options.

Thanks, David
Mike Stump Dec. 24, 2015, 6:15 p.m. UTC | #4
On Dec 22, 2015, at 9:13 AM, Peter Bergner <bergner@vnet.ibm.com> wrote:
> 	PR target/68772

No, this is wrong.  Compare to 68872 in the subject line.

> 	* config/rs6000/rs6000.h (ASM_CPU_SPEC): For -mcpu=powerpc64le,
> 	pass %(asm_cpu_power8)/-mpwr8.
> 	* config/rs6000/aix53.h: Likewise.
> 	* config/rs6000/aix61.h: Likewise.
> 	* config/rs6000/aix71.h: Likewise.
> 
> gcc/testsuite/
>        PR target/68772

Likewise.  Please retroactively update the number in the changelog.
Peter Bergner Jan. 5, 2016, 5:22 p.m. UTC | #5
On Thu, 2015-12-24 at 10:15 -0800, Mike Stump wrote:
> On Dec 22, 2015, at 9:13 AM, Peter Bergner <bergner@vnet.ibm.com>
> > gcc/testsuite/
> >        PR target/68772
> 
> Likewise.  Please retroactively update the number in the changelog.

Wow, thanks for catching this!  Fixed everywhere.

Peter
diff mbox

Patch

Index: gcc/config/rs6000/rs6000.h
===================================================================
--- gcc/config/rs6000/rs6000.h	(revision 231887)
+++ gcc/config/rs6000/rs6000.h	(working copy)
@@ -128,6 +128,7 @@ 
 %{mcpu=power9: %(asm_cpu_power9)} \
 %{mcpu=a2: -ma2} \
 %{mcpu=powerpc: -mppc} \
+%{mcpu=powerpc64le: %(asm_cpu_power8)} \
 %{mcpu=rs64a: -mppc64} \
 %{mcpu=401: -mppc} \
 %{mcpu=403: -m403} \
Index: gcc/config/rs6000/aix53.h
===================================================================
--- gcc/config/rs6000/aix53.h	(revision 231887)
+++ gcc/config/rs6000/aix53.h	(working copy)
@@ -65,6 +65,7 @@ 
 %{mcpu=power8: -mpwr8} \
 %{mcpu=power9: -mpwr9} \
 %{mcpu=powerpc: -mppc} \
+%{mcpu=powerpc64le: -mpwr8} \
 %{mcpu=rs64a: -mppc} \
 %{mcpu=603: -m603} \
 %{mcpu=603e: -m603} \
Index: gcc/config/rs6000/aix61.h
===================================================================
--- gcc/config/rs6000/aix61.h	(revision 231887)
+++ gcc/config/rs6000/aix61.h	(working copy)
@@ -82,6 +82,7 @@ 
 %{mcpu=power8: -mpwr8} \
 %{mcpu=power9: -mpwr9} \
 %{mcpu=powerpc: -mppc} \
+%{mcpu=powerpc64le: -mpwr8} \
 %{mcpu=rs64a: -mppc} \
 %{mcpu=603: -m603} \
 %{mcpu=603e: -m603} \
Index: gcc/config/rs6000/aix71.h
===================================================================
--- gcc/config/rs6000/aix71.h	(revision 231887)
+++ gcc/config/rs6000/aix71.h	(working copy)
@@ -81,6 +81,7 @@ 
 %{mcpu=power7: -mpwr7} \
 %{mcpu=power8: -mpwr8} \
 %{mcpu=powerpc: -mppc} \
+%{mcpu=powerpc64le: -mpwr8} \
 %{mcpu=rs64a: -mppc} \
 %{mcpu=603: -m603} \
 %{mcpu=603e: -m603} \
Index: gcc/testsuite/gcc.target/powerpc/pr68872.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr68872.c	(revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr68872.c	(working copy)
@@ -0,0 +1,14 @@ 
+/* PR target/68872 */
+/* { dg-do assemble { target { powerpc64le-*-* } } } */
+/* { dg-options "-mcpu=powerpc64le" } */
+
+/* Verify that -mcpu=powerpc64le passes -mpower8/-mpwr8 to the assembler.  */
+
+long
+bar (unsigned char *ptr, unsigned char val)
+{
+  long ret;
+  asm volatile ("stbcx. %0,0,%1" :: "r" (val), "r" (ptr));
+  asm volatile ("mfcr %0,8" : "=r" (ret) ::);
+  return ret;
+}