This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RS6000] Don't pass -many to the assembler


On Wed, Nov 14, 2018 at 01:43:57PM +1030, Alan Modra wrote:
> On Tue, Nov 13, 2018 at 05:17:41AM -0600, Segher Boessenkool wrote:
> > On Tue, Nov 13, 2018 at 12:02:55PM +1030, Alan Modra wrote:
> > > OK, fair enough.  Another option is to just disable -many when gcc is
> > > in development, like we enable checking.
> > 
> > That is a good plan for GCC 9 at least.
> 
> Here's the patch.  Bootstrapped etc. powerpc64le-linux with resultant
> fail of clone2 test as already noted.

Revised again, with a bunch of related issues solved.  Bootstrapped
etc. powerpc64le-linux with no regressions.  OK to apply mainline?

---
I'd like to remove -many from the options passed by default to the
assembler, on the grounds that a gcc bug in instruction selection (eg.
emitting a power9 insn for -mcpu=power8) is better found at assembly
time than run time.

For now, just do this when --enable-checking or gcc is not a release.

In contrast to the previous patch, I haven't changed any of the AIX
header files in this patch.  So AIX gcc will continue to pass -many to
their assembler until someone else (David?) makes that change.  This
patch also emits .machine assembler directives for ELF targets when
functions are compiled for different cpus via attributes or pragmas.
That's necessary when the initial -m<cpu> option passed to the
assembler doesn't enable the superset of all opcodes emitted, as seen
by the earlier failure of gcc.target/powerpc/clone2.c (without
.machine) when building gcc for power8.

O3-pr70130.c also failed on an earlier version of this patch (when
only testing one ISA bit to determine .machine).  This is a test for a
power7 vector bug, but on power8 hw check_vect_support_and_set_flags
passes -mpower8-vector which means the test isn't exercising the
original bug exactly.  I reckon that is wrong, and similary for other
vector testcases that ask for a specific cpu.  I've fixed it here by
explicitly passing -mno-power8-vector and similar vector options.

	* config/rs6000/rs6000.h (ASM_OPT_ANY): Define.
	(ASM_CPU_SPEC): Conditionally add -many.
	* config/rs6000/rs6000.c (rs6000_machine): New static var.
	(rs6000_machine_from_flags, emit_asm_machine): New functions..
	(rs6000_file_start): ..extracted from here, and modified to
	test all ISA bits.
	(rs6000_output_function_prologue): Emit .machine as necessary.
	* testsuite/gcc.target/powerpc/ppc32-abi-dfp-1.c: Don't use
	power mnemonics.
	* testsuite/gcc.dg/vect/O3-pr70130.c: Disable default options
	added by check_vect_support_and_set_flags.
	* testsuite/gcc.dg/vect/pr48765.c: Likewise.
	* testsuite/gfortran.dg/vect/pr45714-b.f: Likewise.

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index f774e2d0bf7..4ca68d0a1d1 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -5715,6 +5715,36 @@ rs6000_builtin_md_vectorized_function (tree fndecl, tree type_out,
 /* Default CPU string for rs6000*_file_start functions.  */
 static const char *rs6000_default_cpu;
 
+#ifdef USING_ELFOS_H
+static const char *rs6000_machine;
+
+static const char *
+rs6000_machine_from_flags (void)
+{
+  if ((rs6000_isa_flags & (ISA_3_0_MASKS_SERVER ^ ISA_2_7_MASKS_SERVER)) != 0)
+    return "power9";
+  if ((rs6000_isa_flags & (ISA_2_7_MASKS_SERVER ^ ISA_2_6_MASKS_SERVER)) != 0)
+    return "power8";
+  if ((rs6000_isa_flags & (ISA_2_6_MASKS_SERVER ^ ISA_2_5_MASKS_SERVER)) != 0)
+    return "power7";
+  if ((rs6000_isa_flags & (ISA_2_5_MASKS_SERVER ^ ISA_2_4_MASKS)) != 0)
+    return "power6";
+  if ((rs6000_isa_flags & (ISA_2_4_MASKS ^ ISA_2_1_MASKS)) != 0)
+    return "power5";
+  if ((rs6000_isa_flags & ISA_2_1_MASKS) != 0)
+    return "power4";
+  if ((rs6000_isa_flags & OPTION_MASK_POWERPC64) != 0)
+    return "ppc64";
+  return "ppc";
+}
+
+static void
+emit_asm_machine (void)
+{
+  fprintf (asm_out_file, "\t.machine %s\n", rs6000_machine);
+}
+#endif
+
 /* Do anything needed at the start of the asm file.  */
 
 static void
@@ -5780,27 +5810,10 @@ rs6000_file_start (void)
     }
 
 #ifdef USING_ELFOS_H
+  rs6000_machine = rs6000_machine_from_flags ();
   if (!(rs6000_default_cpu && rs6000_default_cpu[0])
       && !global_options_set.x_rs6000_cpu_index)
-    {
-      fputs ("\t.machine ", asm_out_file);
-      if ((rs6000_isa_flags & OPTION_MASK_MODULO) != 0)
-	fputs ("power9\n", asm_out_file);
-      else if ((rs6000_isa_flags & OPTION_MASK_DIRECT_MOVE) != 0)
-	fputs ("power8\n", asm_out_file);
-      else if ((rs6000_isa_flags & OPTION_MASK_POPCNTD) != 0)
-	fputs ("power7\n", asm_out_file);
-      else if ((rs6000_isa_flags & OPTION_MASK_CMPB) != 0)
-	fputs ("power6\n", asm_out_file);
-      else if ((rs6000_isa_flags & OPTION_MASK_POPCNTB) != 0)
-	fputs ("power5\n", asm_out_file);
-      else if ((rs6000_isa_flags & OPTION_MASK_MFCRF) != 0)
-	fputs ("power4\n", asm_out_file);
-      else if ((rs6000_isa_flags & OPTION_MASK_POWERPC64) != 0)
-	fputs ("ppc64\n", asm_out_file);
-      else
-	fputs ("ppc\n", asm_out_file);
-    }
+    emit_asm_machine ();
 #endif
 
   if (DEFAULT_ABI == ABI_ELFv2)
@@ -27757,7 +27770,17 @@ static void
 rs6000_output_function_prologue (FILE *file)
 {
   if (!cfun->is_thunk)
-    rs6000_output_savres_externs (file);
+    {
+      rs6000_output_savres_externs (file);
+#ifdef USING_ELFOS_H
+      const char *curr_machine = rs6000_machine_from_flags ();
+      if (rs6000_machine != curr_machine)
+	{
+	  rs6000_machine = curr_machine;
+	  emit_asm_machine ();
+	}
+#endif
+    }
 
   /* ELFv2 ABI r2 setup code and local entry point.  This must follow
      immediately after the global entry point label.  */
diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h
index e7e998d1492..2e2d253705b 100644
--- a/gcc/config/rs6000/rs6000.h
+++ b/gcc/config/rs6000/rs6000.h
@@ -70,6 +70,12 @@
 #define PPC405_ERRATUM77 0
 #endif
 
+#if CHECKING_P
+#define ASM_OPT_ANY ""
+#else
+#define ASM_OPT_ANY " -many"
+#endif
+
 /* Common ASM definitions used by ASM_SPEC among the various targets for
    handling -mcpu=xxx switches.  There is a parallel list in driver-rs6000.c to
    provide the default assembler options if the user uses -mcpu=native, so if
@@ -137,8 +143,8 @@
 	    mvsx: -mpower7; \
 	    mpowerpc64: -mppc64;: %(asm_default)}; \
   :%eMissing -mcpu option in ASM_CPU_SPEC?\n} \
-%{mvsx: -mvsx -maltivec; maltivec: -maltivec} \
--many"
+%{mvsx: -mvsx -maltivec; maltivec: -maltivec}" \
+ASM_OPT_ANY
 
 #define CPP_DEFAULT_SPEC ""
 
diff --git a/gcc/testsuite/gcc.dg/vect/O3-pr70130.c b/gcc/testsuite/gcc.dg/vect/O3-pr70130.c
index 18a295c83f0..f8b84405140 100644
--- a/gcc/testsuite/gcc.dg/vect/O3-pr70130.c
+++ b/gcc/testsuite/gcc.dg/vect/O3-pr70130.c
@@ -1,5 +1,5 @@
 /* { dg-require-effective-target vsx_hw { target powerpc*-*-* } } */
-/* { dg-additional-options "-mcpu=power7" { target powerpc*-*-* } } */
+/* { dg-additional-options "-mcpu=power7 -mno-power9-vector -mno-power8-vector" { target powerpc*-*-* } } */
 
 #include "tree-vect.h"
 
diff --git a/gcc/testsuite/gcc.dg/vect/pr48765.c b/gcc/testsuite/gcc.dg/vect/pr48765.c
index ae364379d07..b091a145d0f 100644
--- a/gcc/testsuite/gcc.dg/vect/pr48765.c
+++ b/gcc/testsuite/gcc.dg/vect/pr48765.c
@@ -1,6 +1,6 @@
 /* { dg-do compile { target { powerpc*-*-* } } } */
 /* { dg-skip-if "do not override -mcpu" { *-*-* } { "-mcpu=*" } { "-mcpu=power6" } } */
-/* { dg-additional-options "-O3 -mcpu=power6" } */
+/* { dg-additional-options "-O3 -mcpu=power6 -mno-power9-vector -mno-power8-vector -mno-vsx" } */
 
 enum reg_class
 {
diff --git a/gcc/testsuite/gcc.target/powerpc/ppc32-abi-dfp-1.c b/gcc/testsuite/gcc.target/powerpc/ppc32-abi-dfp-1.c
index 14908dba690..eea7f6ffc2e 100644
--- a/gcc/testsuite/gcc.target/powerpc/ppc32-abi-dfp-1.c
+++ b/gcc/testsuite/gcc.target/powerpc/ppc32-abi-dfp-1.c
@@ -45,14 +45,14 @@ __asm__ ("\t.globl\t" #NAME "_asm\n\t"					\
 	 #NAME "_asm:\n\t"						\
 	 "lis 11,gparms@ha\n\t"						\
 	 "la 11,gparms@l(11)\n\t"					\
-	 "st 3,0(11)\n\t"						\
-	 "st 4,4(11)\n\t"						\
-	 "st 5,8(11)\n\t"						\
-	 "st 6,12(11)\n\t"						\
-	 "st 7,16(11)\n\t"						\
-	 "st 8,20(11)\n\t"						\
-	 "st 9,24(11)\n\t"						\
-	 "st 10,28(11)\n\t"						\
+	 "stw 3,0(11)\n\t"						\
+	 "stw 4,4(11)\n\t"						\
+	 "stw 5,8(11)\n\t"						\
+	 "stw 6,12(11)\n\t"						\
+	 "stw 7,16(11)\n\t"						\
+	 "stw 8,20(11)\n\t"						\
+	 "stw 9,24(11)\n\t"						\
+	 "stw 10,28(11)\n\t"						\
 	 "stfd 1,32(11)\n\t"						\
 	 "stfd 2,40(11)\n\t"						\
 	 "stfd 3,48(11)\n\t"						\
diff --git a/gcc/testsuite/gfortran.dg/vect/pr45714-b.f b/gcc/testsuite/gfortran.dg/vect/pr45714-b.f
index 0d00c6fd666..abf33cd25b8 100644
--- a/gcc/testsuite/gfortran.dg/vect/pr45714-b.f
+++ b/gcc/testsuite/gfortran.dg/vect/pr45714-b.f
@@ -1,5 +1,5 @@
 ! { dg-do compile { target powerpc*-*-* } }
-! { dg-additional-options "-O3 -mcpu=power7 -ffast-math -mveclibabi=mass" }
+! { dg-additional-options "-O3 -mcpu=power7 -mno-power9-vector -mno-power8-vector -ffast-math -mveclibabi=mass" }
 
       integer index(18),i,j,k,l,ipiv(18),info,ichange,neq,lda,ldb,
      &  nrhs,iplas

-- 
Alan Modra
Australia Development Lab, IBM


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]