On Tue, Nov 13, 2012 at 10:03 AM, Uros Bizjak <ubiz...@gmail.com> wrote: > On Tue, Nov 13, 2012 at 3:30 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. Thanks. -- H.J.
