Bug 37878 - [4.4 regression] PPC64 ldu command generated with invalid offset
Summary: [4.4 regression] PPC64 ldu command generated with invalid offset
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 4.4.0
: P1 normal
Target Milestone: 4.4.0
Assignee: David Edelsohn
URL: http://gcc.gnu.org/ml/gcc-patches/200...
Keywords: wrong-code
Depends on:
Reported: 2008-10-21 00:31 UTC by lucier
Modified: 2008-10-30 00:02 UTC (History)
3 users (show)

See Also:
Target: powerpc64-*-*
Known to work:
Known to fail:
Last reconfirmed: 2008-10-21 08:36:02

test file input (3.52 KB, text/plain)
2008-10-21 00:32 UTC, lucier
test file output (1.01 KB, text/plain)
2008-10-21 00:33 UTC, lucier
gcc44-pr37878.patch (297 bytes, patch)
2008-10-21 15:10 UTC, Jakub Jelinek
Details | Diff
gcc44-pr37878.patch (419 bytes, patch)
2008-10-22 11:33 UTC, Jakub Jelinek
Details | Diff
disallow illegal displacement wrapped in PRE_MODIFY or PRE_INC/DEC (430 bytes, patch)
2008-10-28 22:35 UTC, David Edelsohn
Details | Diff
another refinement (470 bytes, patch)
2008-10-28 23:13 UTC, David Edelsohn
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description lucier 2008-10-21 00:31:14 UTC
With this compiler:

[descartes:gcc/objdirs/gambc-v4_1_2] lucier% /pkgs/gcc-4.4.0-64/bin/gcc -v
Using built-in specs.
Target: powerpc64-apple-darwin9.5.0
Configured with: ../../mainline/configure CC='/usr/bin/gcc-4.0 -mcpu=970 -m64' --disable-werror --build=powerpc64-apple-darwin9.5.0 --host=powerpc64-apple-darwin9.5.0 --target=powerpc64-apple-darwin9.5.0 --with-gmp-include=/sw/include/ --with-gmp-lib=/sw/lib/ppc64 --with-mpfr-include=/sw/include/ --with-mpfr-lib=/sw/lib/ppc64 --prefix=/pkgs/gcc-4.4.0-64 --with-libiconv-prefix=/usr --with-system-zlib
Thread model: posix
gcc version 4.4.0 20081020 (experimental) [trunk revision 141240] (GCC) 

I get this error:

[descartes:gcc/objdirs/gambc-v4_1_2] lucier% /pkgs/gcc-4.4.0-64/bin/gcc -save-temps -mcpu=970 -m64 -I../include -I. -no-cpp-precomp -Wall -W -Wno-unused -O1 -fno-math-errno -fschedule-insns2 -fno-trapping-math -fno-strict-aliasing -fwrapv -fomit-frame-pointer -fPIC -fno-common -DHAVE_CONFIG_H -D___PRIMAL -D___LIBRARY -c assemtest.i
gcc: unrecognized option '-no-cpp-precomp'
assemtest.s:69:Parameter error: expression must be a multiple of 4 (parameter 2)

The offending assembler command is

        ldu r0,7(r9)

I'll include assemtest.i; the code itself is about 50 lines of machine-generated C code, plus a lot of declarations so I could get it to compile separately.
Comment 1 lucier 2008-10-21 00:32:25 UTC
Created attachment 16517 [details]
test file input

This is the .i file
Comment 2 lucier 2008-10-21 00:33:02 UTC
Created attachment 16518 [details]
test file output
Comment 3 Andreas Schwab 2008-10-21 08:36:02 UTC
Reduced testcase:

double y, z;
void foo (long x)
  y = *(double *) ((long *) (x - 1) + 1);
  z = *(double *) ((long *) (x - 1) + 1);

$ gcc -m64 -O -c ldu.c
/tmp/ccYujYhd.s: Assembler messages:
/tmp/ccYujYhd.s:20: Error: operand out of domain (7 is not a multiple of 4)

The insn is generated during auto-inc-dec.

(insn 9 8 10 2 ldu.c:4 (set (reg:DF 122)
        (mem:DF (pre_modify:DI (reg/f:DI 119 [ D.1253 ])
                (plus:DI (reg/f:DI 119 [ D.1253 ])
                    (const_int 7 [0x7]))) [0 S8 A64])) 345 {*movdf_hardfloat64} (expr_list:REG_INC (reg/f:DI 119 [ D.1253 ])
Comment 4 Jakub Jelinek 2008-10-21 09:55:11 UTC
I think this is rs6000_legitimate_address at fault here, it shouldn't say it is valid memory address.  In the:
if (GET_CODE (x) == PRE_MODIFY block in that function,
rs6000_legitimate_offset_address_p (mode, XEXP (x, 1), reg_ok_strict)
is true because:
3649          if (mode == DFmode || mode == DDmode || !TARGET_POWERPC64)
3650            extra = 4;
3651          else if (offset & 3)
3652            return false;
only checks the offset being multiple of 4 if not DFmode or DDmode.  If that is correct, I think rs6000_legitimate_address should in the PRE_MODIFY case explicitly check that if offset is a constant that it is multiple of 4.
Comment 5 David Edelsohn 2008-10-21 14:53:07 UTC
The test in rs6000_legitimate_offset_address_p is for something completely different.  This should be tested in rs6000_legitimate_address:

  if (GET_CODE (x) == PRE_MODIFY
      && mode != TImode
      && mode != TFmode
      && mode != TDmode
          || TARGET_POWERPC64
          || ((mode != DFmode && mode != DDmode) || TARGET_E500_DOUBLE))
      && (TARGET_POWERPC64 || mode != DImode)
      && !ALTIVEC_VECTOR_MODE (mode)
      && !SPE_VECTOR_MODE (mode)
      /* Restrict addressing for DI because of our SUBREG hackery.  */
      && !(TARGET_E500_DOUBLE
           && (mode == DFmode || mode == DDmode || mode == DImode))
      && legitimate_indirect_address_p (XEXP (x, 0), reg_ok_strict)
      && (rs6000_legitimate_offset_address_p (mode, XEXP (x, 1), reg_ok_strict)
          || legitimate_indexed_address_p (XEXP (x, 1), reg_ok_strict))
      && rtx_equal_p (XEXP (XEXP (x, 1), 0), XEXP (x, 0)))
    return 1;
Comment 6 Jakub Jelinek 2008-10-21 15:10:46 UTC
Created attachment 16519 [details]

So something like this?
Comment 7 David Edelsohn 2008-10-21 15:14:44 UTC
Also, note the problem is a DFmode value in a GPR.  FPRs can handle the non-word-aligned offset.  Forcing all DFmode to be 64 bit aligned is not an option because that is known to impose a severe performance penalty.  For non-update form, we have handled this in rs6000_legitimize_reload_address.
Comment 8 Jakub Jelinek 2008-10-22 11:33:36 UTC
Created attachment 16526 [details]

Just for the record, after discussions on IRC the bug was identified in
Comment 9 lucier 2008-10-23 19:20:49 UTC
I bootstrapped and regtested the suggested patch.  There was one fewer FAIL in the gcc tests:

FAIL: gcc.c-torture/execute/nestfunc-6.c execution,  -O0 

and one more failure in the libgomp tests:

FAIL: libgomp.fortran/crayptr2.f90  -O3 -fomit-frame-pointer -funroll-loops  execution test

However, it's not clear to me from the output of gdb implies  that this may is a problem with the compiled code (the command lines are taken from the log file):

[descartes:powerpc64-apple-darwin9.5.0/libgomp/testsuite] lucier% /Users/lucier/programs/gcc/objdirs/mainline/gcc/xgcc -B/Users/lucier/programs/gcc/objdirs/mainline/gcc/ /Users/lucier/programs/gcc/mainline/libgomp/testsuite/libgomp.fortran/crayptr2.f90  -B/Users/lucier/programs/gcc/objdirs/mainline/powerpc64-apple-darwin9.5.0/./libgomp/ -I/Users/lucier/programs/gcc/objdirs/mainline/powerpc64-apple-darwin9.5.0/./libgomp -I/Users/lucier/programs/gcc/mainline/libgomp/testsuite/.. -shared-libgcc -fmessage-length=0 -fopenmp  -O3 -fomit-frame-pointer -funroll-loops  -fopenmp -fcray-pointer -static-libgcc   -L/Users/lucier/programs/gcc/objdirs/mainline/powerpc64-apple-darwin9.5.0/./libgomp/.libs -lgomp -L/Users/lucier/programs/gcc/objdirs/mainline/powerpc64-apple-darwin9.5.0/./libgomp/../libgfortran/.libs -lgfortranbegin -lgfortran -lm   -mcpu=970 -m64 -o ./crayptr2.exe
[descartes:powerpc64-apple-darwin9.5.0/libgomp/testsuite] lucier% env LD_LIBRARY_PATH=.:/Users/lucier/programs/gcc/objdirs/mainline/powerpc64-apple-darwin9.5.0/./libgomp/.libs:/Users/lucier/programs/gcc/objdirs/mainline/gcc:/Users/lucier/programs/gcc/objdirs/mainline/powerpc64-apple-darwin9.5.0/./libgomp/../libgfortran/.libs:.:/Users/lucier/programs/gcc/objdirs/mainline/powerpc64-apple-darwin9.5.0/./libgomp/.libs:/Users/lucier/programs/gcc/objdirs/mainline/gcc:/Users/lucier/programs/gcc/objdirs/mainline/powerpc64-apple-darwin9.5.0/./libgomp/../libgfortran/.libs gdb ./crayptr2.exe
GNU gdb 6.3.50-20050815 (Apple version gdb-962) (Sat Jul 26 08:17:57 UTC 2008)
Copyright 2004 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you are
welcome to change it and/or distribute copies of it under certain conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB.  Type "show warranty" for details.
This GDB was configured as "powerpc-apple-darwin"...Reading symbols for shared libraries .... done

(gdb) run
Starting program: /Users/lucier/programs/gcc/objdirs/mainline/powerpc64-apple-darwin9.5.0/libgomp/testsuite/crayptr2.exe 
warning: posix_spawn failed, trying execvp, error: 86
Reading symbols for shared libraries +++.. done

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x0000000000000000
0x0000000100001678 in MAIN__.omp_fn.0 ()
(gdb) where
#0  0x0000000100001678 in MAIN__.omp_fn.0 ()
#1  0x000000010000187c in MAIN__ ()
#2  0x00000001000018e4 in main (argc=1, argv=<value temporarily unavailable, due to optimizations>) at ../../../../mainline/libgfortran/fmain.c:21

It is completely reproducible, however.

Comment 10 lucier 2008-10-23 21:25:31 UTC
This patch fixes my original problem and the reduced test case.

The two testresults reports I referred to earlier are at





Comment 11 David Edelsohn 2008-10-28 22:35:47 UTC
Created attachment 16576 [details]
disallow illegal displacement wrapped in PRE_MODIFY or PRE_INC/DEC

I think this patch accomplishes the effect but easier to follow with the address form localized in one place.
Comment 12 David Edelsohn 2008-10-28 23:13:56 UTC
Created attachment 16577 [details]
another refinement

Jakub suggested a further refinement to hoist a common sub-expression.
Comment 13 David Edelsohn 2008-10-29 23:33:17 UTC
Subject: Bug 37878

Author: dje
Date: Wed Oct 29 23:33:02 2008
New Revision: 141450

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=141450
        PR target/37878
        * config/rs6000/predicates.md (word_offset_memref_operand):
        Restructure code and look inside auto-inc/dec addresses.


Comment 14 lucier 2008-10-30 00:02:37 UTC
Thank you, this fixes the original bug.

I took the liberty of closing this bug report.

Thanks again.