Bug 54457 - [x32] Fail to combine 64bit index + constant
Summary: [x32] Fail to combine 64bit index + constant
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: rtl-optimization (show other bugs)
Version: 4.8.0
: P3 normal
Target Milestone: 4.8.0
Assignee: Uroš Bizjak
URL: http://gcc.gnu.org/ml/gcc-patches/201...
Keywords:
Depends on:
Blocks:
 
Reported: 2012-09-02 13:19 UTC by H.J. Lu
Modified: 2012-11-01 22:37 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2012-09-24 00:00:00


Attachments
Proposed patch (514 bytes, patch)
2012-09-24 22:48 UTC, Uroš Bizjak
Details | Diff
Updated patch (931 bytes, patch)
2012-09-25 00:00 UTC, Uroš Bizjak
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description H.J. Lu 2012-09-02 13:19:58 UTC
[hjl@gnu-ivb-1 pr54445]$ cat z.i
extern char array[40];

char
foo (long long int position) {
  return array[position + 1];
}
[hjl@gnu-ivb-1 pr54445]$ /export/build/gnu/gcc-x32/build-x86_64-linux/prev-gcc/xgcc -B/export/build/gnu/gcc-x32/build-x86_64-linux/prev-gcc/ -Wall -O2 -std=gnu11 -mx32 -S z.i -da
[hjl@gnu-ivb-1 pr54445]$ cat z.s
	.file	"z.i"
	.text
	.p2align 4,,15
	.globl	foo
	.type	foo, @function
foo:
.LFB0:
	.cfi_startproc
	addq	$1, %rdi
	movzbl	array(%edi), %eax
	ret
	.cfi_endproc
.LFE0:
	.size	foo, .-foo
	.ident	"GCC: (GNU) 4.8.0 20120901 (experimental)"
	.section	.note.GNU-stack,"",@progbits
[hjl@gnu-ivb-1 pr54445]$ 

combine fails on:

Trying 6 -> 8:
Failed to match this instruction:
(set (reg:QI 66)
    (mem/j:QI (plus:SI (subreg:SI (plus:DI (reg/v:DI 62 [ position ])
                    (const_int 1 [0x1])) 0)
            (symbol_ref:SI ("array") [flags 0x40]  <var_decl 0x7ffff19ad260 arra
y>)) [0 array S1 A8]))

This should be a valid address.
Comment 1 Uroš Bizjak 2012-09-24 18:38:46 UTC
(In reply to comment #0)

> combine fails on:
> 
> Trying 6 -> 8:
> Failed to match this instruction:
> (set (reg:QI 66)
>     (mem/j:QI (plus:SI (subreg:SI (plus:DI (reg/v:DI 62 [ position ])
>                     (const_int 1 [0x1])) 0)
>             (symbol_ref:SI ("array") [flags 0x40]  <var_decl 0x7ffff19ad260
> arra
> y>)) [0 array S1 A8]))
> 
> This should be a valid address.

In principle yes, but the RTX is not accepted in ix86_decompose_address since we have two displacements here. Combine should simplify this RTX to:

(set (reg:QI 68)
    (mem/j:QI (plus:SI (subreg:SI (reg/v:DI 62 [ position ]) 0)
            (const:SI (plus:SI (symbol_ref:SI ("array") [flags 0x40]  <var_decl 0x7f8d1bc41390 array>)
                    (const_int 1 [0x1])))) [0 array S1 A8]))


as is the case with -m32 (but rejected in ix86_address_subreg_operand):

  /* Don't allow SUBREGs that span more than a word.  It can lead to spill
     failures when the register is one word out of a two word structure.  */
  if (GET_MODE_SIZE (mode) > UNITS_PER_WORD)
    return false;
Comment 2 Uroš Bizjak 2012-09-24 22:48:37 UTC
Created attachment 28261 [details]
Proposed patch

Patch that enhances combine_simplify_rtx to generate canonical binop sequences that simplify_plus_minus can recognize and further optimize.

Bootstrapped and regression tested on x86_64-pc-linux-gnu.

Patched gcc generates expected code for the above testcase:

foo:
        movzbl  array+1(%edi), %eax
        ret
Comment 3 Uroš Bizjak 2012-09-25 00:00:51 UTC
Created attachment 28262 [details]
Updated patch
Comment 4 Uroš Bizjak 2012-09-25 07:29:32 UTC
Patch at [1].

[1] http://gcc.gnu.org/ml/gcc-patches/2012-09/msg01682.html
Comment 5 uros 2012-10-01 15:00:50 UTC
Author: uros
Date: Mon Oct  1 15:00:41 2012
New Revision: 191928

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=191928
Log:
        PR rtl-optimization/54457
        * simplify-rtx.c (simplify_subreg):
	Simplify (subreg:M (op:N ((x:N) (y:N)), 0)
     	to (op:M (subreg:M (x:N) 0) (subreg:M (x:N) 0)), where
	the outer subreg is effectively a truncation to the original mode M.

testsuite/ChangeLog:

        PR rtl-optimization/54457
        * gcc.target/i386/pr54457.c: New test.


Added:
    trunk/gcc/testsuite/gcc.target/i386/pr54457.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/simplify-rtx.c
    trunk/gcc/testsuite/ChangeLog
Comment 6 Uroš Bizjak 2012-10-02 13:31:34 UTC
Fixed.
Comment 7 Jack Howarth 2012-11-01 13:49:44 UTC
Shouldn't the gcc.target/i386/pr54457.c testcase be...

Index: gcc/testsuite/gcc.target/i386/pr54457.c
===================================================================
--- gcc/testsuite/gcc.target/i386/pr54457.c     (revision 193061)
+++ gcc/testsuite/gcc.target/i386/pr54457.c     (working copy)
@@ -1,4 +1,4 @@
-/* { dg-do compile { target { ! { ia32 } } } } */
+/* { dg-do compile { target { ! { ia32 || lp64 } } } } */
 /* { dg-options "-O2 -mx32 -maddress-mode=short" } */
 
 extern char array[40];

On x86_64-apple-darwin12 at -m64, the gcc.target/i386/pr54457.c testcase fails the excessive errors test whereas the change above converts that to an unsupported test.
Comment 8 H.J. Lu 2012-11-01 22:31:45 UTC
(In reply to comment #7)
> Shouldn't the gcc.target/i386/pr54457.c testcase be...
> 
> Index: gcc/testsuite/gcc.target/i386/pr54457.c
> ===================================================================
> --- gcc/testsuite/gcc.target/i386/pr54457.c     (revision 193061)
> +++ gcc/testsuite/gcc.target/i386/pr54457.c     (working copy)
> @@ -1,4 +1,4 @@
> -/* { dg-do compile { target { ! { ia32 } } } } */
> +/* { dg-do compile { target { ! { ia32 || lp64 } } } } */
>  /* { dg-options "-O2 -mx32 -maddress-mode=short" } */
> 
>  extern char array[40];
> 
> On x86_64-apple-darwin12 at -m64, the gcc.target/i386/pr54457.c testcase fails
> the excessive errors test whereas the change above converts that to an
> unsupported test.

It will disable test on Linux/x86-64.  We can add
check_effective_target_maybe_x32 to check if -mx32
works before compiling with -mx32.
Comment 9 H.J. Lu 2012-11-01 22:37:20 UTC
Like this:

# Return 1 if -mx32 can compile, 0 otherwise.

proc check_effective_target_maybe_x32 { } {
    return [check_no_compiler_messages maybe_x32 object {
        void foo (void) {}
    } {-mx32}]
}