On Tue, Nov 13, 2012 at 7:23 PM, H.J. Lu <hjl.to...@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 <ebotca...@adacore.com> >>> H.J. Lu <hongjiu...@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...@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... ). > > That is true. > >> 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'. > > Will do. > >> BTW: Do we need this patch also for 4.7? x32 address-mode is long by >> default there, but the problem doesn't trigger. >> > > This regression is triggered by revision 188008: > > http://gcc.gnu.org/ml/gcc-cvs/2012-05/msg00038.html > > aka the un-sign-extension of sizetype constants. We can > backport it to 4.7 branch if we want to be on the safer side. Yes, please backport the patch to 4.7 after it lives a couple of days in mainline without problems. Thanks, Uros.
