Patch: PR target/55142: [4.8 Regression] internal compiler error: in plus_constant, at explow.c:88

[Uros Bizjak]

On Tue, Nov 13, 2012 at 3:30 PM, H.J. Lu <hjl.tools@gmail.com> wrote:

>> >>>> Since x32 runs in 64-bit mode, for address -0x40000300(%rax), hardware
>> >>>> sign-extends displacement from 32-bits to 64-bits and adds it to %rax.
>> >>>> But x32 wants 32-bit -0x40000300, not 64-bit -0x40000300.  This patch
>> >>>> uses 32-bit registers instead of 64-bit registers when displacement
>> >>>> < -16*1024*1024.  -16*1024*1024 is used instead of 0 so that we will
>> >>>> still generate -16(%rsp) instead of -16(%esp).
>> >>>>
>> >>>> Tested it on Linux/x32.  OK to install?
>> >>>
>> >>> This problem uncovers a bug in the middle-end, so I guess it would be
>> >>> better to fix it there.
>> >>>
>> >>> Uros.
>> >>
>> >> The problem is it isn't safe to transform
>> >>
>> >> (zero_extend:DI (plus:SI (FOO:SI) (const_int Y)))
>> >>
>> >> to
>> >>
>> >> (plus:DI (zero_extend:DI (FOO:SI)) (const_int Y))
>> >>
>> >> when Y is negative and its absolute value is greater than FOO.  However,
>> >> we have to do this transformation since other parts of GCC depend on
>> >> it.  If we revert the fix for
>> >>
>> >> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49721
>> >>
>> >> we will get
>> >>
>> >> FAIL: gcc.c-torture/compile/990523-1.c  -O3 -g  (internal compiler error)
>> >> FAIL: gcc.c-torture/compile/990523-1.c  -O3 -g  (test for excess errors)
>> >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -fomit-frame-pointer -funroll-all-loo
>> >> ps -finline-functions  (internal compiler error)
>> >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -fomit-frame-pointer -funroll-all-loo
>> >> ps -finline-functions  (test for excess errors)
>> >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -fomit-frame-pointer -funroll-loops
>> >> (internal compiler error)
>> >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -fomit-frame-pointer -funroll-loops
>> >> (test for excess errors)
>> >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -fomit-frame-pointer  (internal compi
>> >> ler error)
>> >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -fomit-frame-pointer  (test for exces
>> >> s errors)
>> >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -g  (internal compiler error)
>> >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -g  (test for excess errors)
>> >> FAIL: gcc.dg/Warray-bounds.c (internal compiler error)
>> >> FAIL: gcc.dg/Warray-bounds.c (test for excess errors)
>> >>
>> >> since we generate pseudo registers to convert SImode to DImode
>> >> after reload.  Fixing it requires significant changes.
>> >>
>> >> This is only a problem for 64-bit register address since the symbolic
>> >> address is 32-bit.  Using 32-bit base/index registers will work around
>> >> this issue.
>> >
>> > This address
>> >
>> > (plus:DI (zero_extend:DI (FOO:SI)) (const_int Y))
>> >
>> > is OK for x32 as long as Y, which is encoded as 32-bit immediate,
>> > is zero-extend from 32-bit to 64-bit.  SImode address does it.
>> > My patch optimizes it a little bit by using SImode address only
>> > for Y < -16*1024*1024.
>>
>> I was wondering, why we operate with constant -16*1024*1024? Should we
>> use 0x7FFFFFF instead? Since the MSB is always zero, we are safe.
>>
>
> We can check 0x7FFFFFF, i.e., disp < 0.  I use -16*1024*1024, which
> is also used to check legitimate address displacements for PIC, to
> reduce code sizes for small negative displacements.  Or we can always
> encode negative displacements with zero-extension, including -1(%rsp).
>
>> Please add a fat ??? comment, why we paper-over this issue and repost
>> the latest patch. I got lost in all the versions :(
>>
>
> Here is the updated patch.
>
> gcc/
>
> 2012-11-13  Eric Botcazou  <ebotcazou@adacore.com>
>             H.J. Lu  <hongjiu.lu@intel.com>
>
>         PR target/55142
>         * config/i386/i386.c (legitimize_pic_address): Properly handle
>         REG + CONST.
>         (ix86_print_operand_address): For x32, zero-extend negative
>         displacement if it < -16*1024*1024.
>
> gcc/testsuite/
>
> 2012-11-13  H.J. Lu  <hongjiu.lu@intel.com>
>
>         PR target/55142
>         * gcc.target/i386/pr55142-1.c: New file.
>         * gcc.target/i386/pr55142-2.c: Likewise.

OK, for mainline SVN (with the reservation that middle-end fix was not
found to be a viable solution, so target fix is papering-over real
issue. Let's wait for the next victim... ).

Oh, and please fix a typo of mine in the one line above the patch
hunk; the modifier for SI addresses should be 'k', not 'l'.

BTW: Do we need this patch also for 4.7? x32 address-mode is long by
default there, but the problem doesn't trigger.

Thanks,
Uros.