Re: [PATCH] rs6000: Disable generation of lwa in 32-bit mode

[Peter Bergner]

On Fri, 2012-10-26 at 15:00 +1030, Alan Modra wrote:
 On Thu, Oct 25, 2012 at 03:57:38PM -0700, Segher Boessenkool wrote:
  for most others.  This patch disables all lwa insns in 32-bit mode.
  We can later re-enable it if the assembler used handles it properly,
 
 Well, you can now do that.  Mainline gas and ld are now fixed.

Now that gas and ld are fixed, can we use a configure time check to
see whether we're using a fixed gas/ld or not before we disable them?

Peter

Re: [PATCH] rs6000: Disable generation of lwa in 32-bit mode

[Segher Boessenkool]

for most others.  This patch disables all lwa insns in 32-bit mode.
We can later re-enable it if the assembler used handles it properly,


Well, you can now do that.  Mainline gas and ld are now fixed.


Yes, you are much too quick for me to keep up.  Thank you!


Now that gas and ld are fixed, can we use a configure time check to
see whether we're using a fixed gas/ld or not before we disable them?


When I wrote this patch, I thought the Darwin assembler had the same
problem.  No idea how I got that in my mind; it doesn't.  And AIX does
not support -m32 -mpowerpc64 at all (the compiler disallows it), so
that is out of the picture as well.

I now bootstrapped and tested with the new binutils.  Most of the
testcases that were fixed with my patch are fixed there as well; and
some (20040709-2.c, etc.) fail with a linker error now, instead of
the runtime error.  Some fail that passed without either patch, but
failing is correct -- wrong code sometimes runs fine :-)

So I'm not applying this patch.  I don't think we need a configure
check: we have had many releases with these bugs already, one more
won't hurt.  People generally use a recent version of binutils as well.


Segher

[PATCH] rs6000: Disable generation of lwa in 32-bit mode

[Segher Boessenkool]

Many (most? all?) assemblers (and really the 32-bit ABIs) do not handle
lwa and ld properly.  Those instructions have a 14-bit offset field, and
the low two bits of the instruction are the extended opcode.  But the
32-bit toolchains use a 16-bit offset relocation, clobbering the low
two bits.  See PR27619 and binutils #14758.

This works out fine for the most common case (word-aligned ld), but not
for most others.  This patch disables all lwa insns in 32-bit mode.
We can later re-enable it if the assembler used handles it properly,
and for lwax and lwaux, and fix the various ld patterns as well, but
this is at least a small step in the right direction.

gcc:
-# of expected passes   108499
-# of unexpected failures   303
+# of expected passes   108508
+# of unexpected failures   295

gfortran:
-# of expected passes   40555
-# of unexpected failures   175
+# of expected passes   40582
+# of unexpected failures   148

The lwa pattern must be below the extsw pattern, because the lwa_operand
predicate allows registers as well.  Changing lwa_operand causes other
problems I do not want to deal with right now.

Bootstrapped and tested on powerpc64-linux -m64,-m32,-m32/-mpowerpc64.

Okay to apply?


Segher


2012-10-25  Segher Boessenkool  seg...@kernel.crashing.org

gcc/
* config/rs6000/rs6000.md (sign_extend:SI patterns): Split
the memory case off.  Merge the two register cases.  Change
the condition for the memory case to require 64-bit mode.

---
 gcc/config/rs6000/rs6000.md |   22 --
 1 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index 2625bd7..5ce7963 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -519,18 +519,9 @@ (define_expand extendsidi2
   )
 
 (define_insn 
-  [(set (match_operand:DI 0 gpc_reg_operand =r,r)
-   (sign_extend:DI (match_operand:SI 1 lwa_operand m,r)))]
-  TARGET_POWERPC64  rs6000_gen_cell_microcode
-  @
-   lwa%U1%X1 %0,%1
-   extsw %0,%1
-  [(set_attr type load_ext,exts)])
-
-(define_insn 
   [(set (match_operand:DI 0 gpc_reg_operand =r)
(sign_extend:DI (match_operand:SI 1 gpc_reg_operand r)))]
-  TARGET_POWERPC64  !rs6000_gen_cell_microcode
+  TARGET_POWERPC64
   extsw %0,%1
   [(set_attr type exts)])
 
@@ -586,6 +577,17 @@ (define_split
(const_int 0)))]
   )
 
+; The 32-bit ABI has no relocation for DS_LO; the assembler uses a LO instead
+; for the @l an lwa instruction uses, overwriting the low two bits in the
+; opcode.  We could do tricky tricks like adding 2 to the offset, but let's
+; not: only allow this instruction in 64-bit mode.
+(define_insn 
+  [(set (match_operand:DI 0 gpc_reg_operand =r)
+   (sign_extend:DI (match_operand:SI 1 lwa_operand m)))]
+  TARGET_64BIT  rs6000_gen_cell_microcode
+  lwa%U1%X1 %0,%1
+  [(set_attr type load_ext)])
+
 (define_expand zero_extendqisi2
   [(set (match_operand:SI 0 gpc_reg_operand )
(zero_extend:SI (match_operand:QI 1 gpc_reg_operand )))]
-- 
1.7.7.6

Re: [PATCH] rs6000: Disable generation of lwa in 32-bit mode

[David Edelsohn]

On Thu, Oct 25, 2012 at 6:57 PM, Segher Boessenkool
seg...@kernel.crashing.org wrote:

 2012-10-25  Segher Boessenkool  seg...@kernel.crashing.org

 gcc/
 * config/rs6000/rs6000.md (sign_extend:SI patterns): Split
 the memory case off.  Merge the two register cases.  Change
 the condition for the memory case to require 64-bit mode.

This is okay.

Thanks, David

Re: [PATCH] rs6000: Disable generation of lwa in 32-bit mode

[Alan Modra]

On Thu, Oct 25, 2012 at 03:57:38PM -0700, Segher Boessenkool wrote:
 for most others.  This patch disables all lwa insns in 32-bit mode.
 We can later re-enable it if the assembler used handles it properly,

Well, you can now do that.  Mainline gas and ld are now fixed.

-- 
Alan Modra
Australia Development Lab, IBM