Bug 72802 - powerpc64le: -mcpu=power9 emits lxssp instruction with offset that isn't a multiple of 4
Summary: powerpc64le: -mcpu=power9 emits lxssp instruction with offset that isn't a mu...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 7.0
: P3 normal
Target Milestone: 6.2
Assignee: Alan Modra
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-08-04 02:52 UTC by Anton Blanchard
Modified: 2018-07-23 02:28 UTC (History)
4 users (show)

See Also:
Host:
Target: powerpc64le-linux
Build:
Known to work:
Known to fail:
Last reconfirmed: 2016-08-04 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Anton Blanchard 2016-08-04 02:52:26 UTC
I just hit this binutils error:

blah.s:401: Error: operand out of domain (2 is not a multiple of 4)

Which corresponds to:

lxssp 31,2(30)

Alan points out that the bottom two bits are part of the opcode, so it had better be 4 byte aligned.
Comment 1 Alan Modra 2016-08-04 03:22:12 UTC
"o" constraints where "wY" ought to be used.  Also, wY looks to be wrong for 32-bit..
Comment 2 Segher Boessenkool 2016-08-04 14:26:40 UTC
Why is "wY" wrong for 32-bit?  I don't see it?
Comment 3 Alan Modra 2016-08-05 00:58:47 UTC
wY is using mem_operand_gpr which is designed for gpr loads/stores.  When -m32, mem_operand_gpr does not enforce multiple-of-4 offsets.
Comment 4 Segher Boessenkool 2016-08-05 01:09:29 UTC
When -mno-powerpc64, I see.  Thanks.  Yeah this needs some untangling.
Comment 5 Alan Modra 2016-08-08 09:05:48 UTC
Author: amodra
Date: Mon Aug  8 09:05:16 2016
New Revision: 239233

URL: https://gcc.gnu.org/viewcvs?rev=239233&root=gcc&view=rev
Log:
[RS6000] PR72802 part 1, fix constraints for lxssp/stxssp

We can't use "o" constraint for lsxxp/stxssp since those insns have a
DS-form offset field, ie. the bottom two bits of the offset must be 0.
So use "wY" instead, but that leads to finding another problem.

mem_operand_gpr is only suitable for gpr loads/stores since it does
not enforce multiple-of-4 offsets when -m32.  So "wY" can't use
mem_operand_gpr, and the vsx tests in mem_operand_gpr are bogus.

I've deleted offsettable_mem_14bit_operand because it wasn't used
anywhere but in the wY constraint.  Note also that the new wY
constraint doesn't use memory_operand because that is redundant in a
constraint, having already been tested in the predicate.

	PR target/72802
	* config/rs6000/rs6000.c (mem_operand_gpr): Remove vsx dform test.
	(mem_operand_ds_form): New predicate.
	* config/rs6000/rs6000-protos.h (mem_operand_ds_form): Declare.
	* config/rs6000/constraints.md (wY): Use mem_operand_df_form.
	* config/rs6000/predicates.md (offsettable_mem_14bit_operand): Delete.
	* config/rs6000/rs6000.md (f32_lm2, f32_sm2): Use wY for SF.
	(extendsfdf2_fpr): Replace o constraint with wY.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/rs6000/constraints.md
    trunk/gcc/config/rs6000/predicates.md
    trunk/gcc/config/rs6000/rs6000-protos.h
    trunk/gcc/config/rs6000/rs6000.c
    trunk/gcc/config/rs6000/rs6000.md
Comment 6 Alan Modra 2016-08-08 09:07:21 UTC
Author: amodra
Date: Mon Aug  8 09:06:49 2016
New Revision: 239234

URL: https://gcc.gnu.org/viewcvs?rev=239234&root=gcc&view=rev
Log:
[RS6000] PR72802 part 2, reload ICE

After fixing the constraint problem, we hit an "insn does not satisfy
its constraints" with -mno-lra on the following insn, a vector load
from mem which has an invalid offset:
(insn 631 630 1122 12 (set (reg:SF 108 31 [orig:260 pretmp_44 ] [260])
        (mem:SF (plus:DI (reg:DI 30 30 [orig:338 ivtmp.141 ] [338])
                (const_int 2 [0x2])) [5 MEM[base: _1, offset: 2B]+0 S4 A32])) 470 {movsf_hardfloat}
     (nil))

Here are the reload costs for the various alternatives of
movsf_hardfloat:
"=!r, !r,  m,  f, ww, ww, !r,  f, wb,  m, wY, wu,  Z,?wn, ?r,*c*l, !r, *h"
  "r,  m,  r,  f, ww,  j,  j,  m, wY,  f, wb,  Z, wu,  r, wn,   r, *h,  0"
 617 609  17  17   8   8 617   9   8  17  17   8  17  23  23   17 617  17

Notice that the cost for a vector<-vector move (ww,ww) is the same as
the cost for a vector<-mem move (wb,wY or wu,Z).  Since the
vector<-vector move comes first, it is chosen and the mem part of the
insn reloaded.  That just gives another copy of insn 631.

	PR target/72802
	* config/rs6000/rs6000.md (mov<mode>_hardfloat): Sort
	alternatives.  Put loads first, then stores, and reg/reg moves
	within same class later.  Delete attr length.
testsuite/
	* gcc.c-torture/compile/pr72802.c: New.

Added:
    trunk/gcc/testsuite/gcc.c-torture/compile/pr72802.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/rs6000/rs6000.md
    trunk/gcc/testsuite/ChangeLog
Comment 7 Alan Modra 2016-08-08 15:09:19 UTC
Fixed on trunk.  The problem exists on gcc-6 since we have wrong "o" constraints on lsxxp/stxssp there.  Triggered by:

float foo (void *p)
{
  register float f __asm__ ("v0");
  f = *(float *)((char *) p + 2);
  __asm__ ("#%0" : "+wa" (f));
  return f;
}
Comment 8 Alan Modra 2016-08-09 05:44:02 UTC
Author: amodra
Date: Tue Aug  9 05:43:29 2016
New Revision: 239269

URL: https://gcc.gnu.org/viewcvs?rev=239269&root=gcc&view=rev
Log:
[RS6000] PR72802 part 1, fix constraints for lxssp/stxssp

	PR target/72802
	* config/rs6000/rs6000.c (mem_operand_gpr): Remove vsx dform test.
	(mem_operand_ds_form): New predicate.
	* config/rs6000/rs6000-protos.h (mem_operand_ds_form): Declare.
	* config/rs6000/constraints.md (wY): New constraint.
	* config/rs6000/rs6000.md (f32_lm2, f32_sm2): Use wY for SF.
	(extendsfdf2_fpr): Replace o constraint with wY.

Modified:
    branches/gcc-6-branch/gcc/ChangeLog
    branches/gcc-6-branch/gcc/config/rs6000/constraints.md
    branches/gcc-6-branch/gcc/config/rs6000/rs6000-protos.h
    branches/gcc-6-branch/gcc/config/rs6000/rs6000.c
    branches/gcc-6-branch/gcc/config/rs6000/rs6000.md
Comment 9 Alan Modra 2016-08-09 05:45:13 UTC
Author: amodra
Date: Tue Aug  9 05:44:39 2016
New Revision: 239270

URL: https://gcc.gnu.org/viewcvs?rev=239270&root=gcc&view=rev
Log:
[RS6000] PR72802 part 2, reload ICE

	PR target/72802
	* config/rs6000/rs6000.md (mov<mode>_hardfloat): Sort
	alternatives.  Put loads first, then stores, and reg/reg moves
	within same class later.  Delete attr length.
testsuite/
	* gcc.c-torture/compile/pr72802.c: New.

Added:
    branches/gcc-6-branch/gcc/testsuite/gcc.c-torture/compile/pr72802.c
Modified:
    branches/gcc-6-branch/gcc/ChangeLog
    branches/gcc-6-branch/gcc/config/rs6000/rs6000.md
    branches/gcc-6-branch/gcc/testsuite/ChangeLog
Comment 10 Alan Modra 2016-08-09 05:50:34 UTC
Fixed
Comment 11 Iain Sandoe 2018-07-19 12:51:59 UTC
The test case fails on m32 Darwin (since it was added, AFAICT) apparently because it's wrong code or triggers a code-gen bug that doesn't happen to cause most platform assemblers to complain.

This is repeatable on x86-64-linux-gnu.

The test-case is a bit inscrutable, but ..

the object
static a[] 

is used thus 
a[i] = something 

in fn6.

the content is never accessed and no storage is associated with a

but the stores to a are not .. so we have (for x86-64-linux-gnu)
	movl	%edx, a(,%rax,4)

and
$ nm pr72802.o
                 U a
0000000000000000 b b

this assembles for linux and x86-64-darwin (with 'a' showing as an undef), but fails on m32 Darwin which believes that the object should be defined in the TU (since it's static) but then finds it to be undef.

Was the intention for the test?
Comment 12 Alan Modra 2018-07-23 02:28:13 UTC
gcc.c-torture/compile/pr72802.c failed for me (likely with -mcpu=power9) with the version of gcc I happened to have at the time I developed the patch in #c5.  I'm not sure now whether it was to demonstrate the #c6 failure or that in #c0, probably the latter since it looks like something I may have been given by Anton (reduced and anonymized from proprietary code).  Unfortunately that testcase doesn't seem to fail with rev239232 or rev239233, and I have no idea what version did fail.  As far as I'm concerned you can delete the testcase..