public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* PATCH: PR rtl-optimization/54157: [x32] -maddress-mode=long failures
@ 2012-08-01 18:41 H.J. Lu
  2012-08-01 18:59 ` Richard Sandiford
  0 siblings, 1 reply; 12+ messages in thread
From: H.J. Lu @ 2012-08-01 18:41 UTC (permalink / raw)
  To: gcc-patches; +Cc: Richard Sandiford

Hi,

We have

(gdb) r -fpreprocessed x.i -quiet -dumpbase x.i -mx32
-maddress-mode=long -mtune=generic -march=x86-64 -auxbase x -O2 -version
-ftree-vectorize -o x.s
Starting program: /export/build/gnu/gcc-x32/build-x86_64-linux/gcc/cc1
-fpreprocessed x.i -quiet -dumpbase x.i -mx32 -maddress-mode=long
-mtune=generic -march=x86-64 -auxbase x -O2 -version -ftree-vectorize -o
x.s
GNU C (GCC) version 4.8.0 20120801 (experimental)
(x86_64-unknown-linux-gnu)
	compiled by GNU C version 4.7.1 20120629 (Red Hat 4.7.1-1), GMP
version 5.0.2, MPFR version 3.1.0, MPC version 0.9
GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096
GNU C (GCC) version 4.8.0 20120801 (experimental)
(x86_64-unknown-linux-gnu)
	compiled by GNU C version 4.7.1 20120629 (Red Hat 4.7.1-1), GMP
version 5.0.2, MPFR version 3.1.0, MPC version 0.9
GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096
Compiler executable checksum: 07a4e516c4e8fe4abfdafa83737d8f4a

Breakpoint 1, fancy_abort (
    file=0x130fe68 "/export/gnu/import/git/gcc/gcc/explow.c", line=88, 
    function=0x131032e <__FUNCTION__.39220> "plus_constant")
    at /export/gnu/import/git/gcc/gcc/diagnostic.c:1011
1011	  internal_error ("in %s, at %s:%d", function, trim_filename
(file), line);
(gdb) f 1
#1  0x0000000000743e07 in plus_constant (mode=DImode, x=0x7ffff106a7e0, 
    c=99452) at /export/gnu/import/git/gcc/gcc/explow.c:88
88	  gcc_assert (GET_MODE (x) == VOIDmode || GET_MODE (x) == mode);
(gdb) f 2
#2  0x0000000000adc4b1 in simplify_binary_operation_1 (code=PLUS,
mode=DImode, 
    op0=0x7ffff106a7e0, op1=0x7ffff1010e80, trueop0=0x7ffff106a7e0, 
    trueop1=0x7ffff1010e80)
    at /export/gnu/import/git/gcc/gcc/simplify-rtx.c:1956
1956		return plus_constant (mode, op0, INTVAL (op1));
(gdb) call debug_rtx (op0)
(symbol_ref:SI ("tmp2") <var_decl 0x7ffff0f06140 tmp2>)
(gdb) call debug_rtx (op1)
(const_int 99452 [0x1847c])
(gdb) bt
#0  fancy_abort (file=0x130fe68
"/export/gnu/import/git/gcc/gcc/explow.c", 
    line=88, function=0x131032e <__FUNCTION__.39220> "plus_constant")
    at /export/gnu/import/git/gcc/gcc/diagnostic.c:1011
#1  0x0000000000743e07 in plus_constant (mode=DImode, x=0x7ffff106a7e0, 
    c=99452) at /export/gnu/import/git/gcc/gcc/explow.c:88
#2  0x0000000000adc4b1 in simplify_binary_operation_1 (code=PLUS,
mode=DImode, 
    op0=0x7ffff106a7e0, op1=0x7ffff1010e80, trueop0=0x7ffff106a7e0, 
    trueop1=0x7ffff1010e80)
    at /export/gnu/import/git/gcc/gcc/simplify-rtx.c:1956
#3  0x0000000000adc221 in simplify_binary_operation (code=PLUS,
mode=DImode, 
    op0=0x7ffff106a7e0, op1=0x7ffff1010e80)
    at /export/gnu/import/git/gcc/gcc/simplify-rtx.c:1904
#4  0x0000000000ae4beb in simplify_plus_minus (code=PLUS, mode=DImode, 
    op0=0x7ffff1071d80, op1=0x7ffff1072440)
    at /export/gnu/import/git/gcc/gcc/simplify-rtx.c:4083
#5  0x0000000000adcd81 in simplify_binary_operation_1 (code=PLUS,
mode=DImode, 
    op0=0x7ffff1071d80, op1=0x7ffff1072440, trueop0=0x7ffff1071d80, 
    trueop1=0x7ffff1072440)
    at /export/gnu/import/git/gcc/gcc/simplify-rtx.c:2079
#6  0x0000000000adc221 in simplify_binary_operation (code=PLUS,
mode=DImode, 
    op0=0x7ffff1071d80, op1=0x7ffff1072440)
    at /export/gnu/import/git/gcc/gcc/simplify-rtx.c:1904
#7  0x0000000000ad6f55 in simplify_gen_binary (code=PLUS, mode=DImode, 
---Type <return> to continue, or q <return> to quit---
    op0=0x7ffff1071d80, op1=0x7ffff1072440)
    at /export/gnu/import/git/gcc/gcc/simplify-rtx.c:173
#8  0x000000000111b868 in force_to_mode (x=0x7ffff1071cd8, mode=DImode, 
    mask=15, just_select=0) at
/export/gnu/import/git/gcc/gcc/combine.c:8392
#9  0x0000000001118ed6 in make_extraction (mode=SImode,
inner=0x7ffff1071cd8, 
    pos=2, pos_rtx=0x0, len=2, unsignedp=1, in_dest=0, in_compare=0)
    at /export/gnu/import/git/gcc/gcc/combine.c:7474
#10 0x0000000001119981 in make_compound_operation (x=0x7ffff1071d68, 
    in_code=SET) at /export/gnu/import/git/gcc/gcc/combine.c:7721
#11 0x0000000001116cc5 in simplify_set (x=0x7ffff1066c78)
    at /export/gnu/import/git/gcc/gcc/combine.c:6539
#12 0x0000000001114e91 in combine_simplify_rtx (x=0x7ffff1066c78, 
    op0_mode=VOIDmode, in_dest=0, in_cond=0)
    at /export/gnu/import/git/gcc/gcc/combine.c:5971
#13 0x0000000001113250 in subst (x=0x7ffff1066c78, from=0x7ffff106a860, 
    to=0x7ffff1071cd8, in_dest=0, in_cond=0, unique_copy=0)
    at /export/gnu/import/git/gcc/gcc/combine.c:5301
#14 0x0000000001112d30 in subst (x=0x7ffff1059870, from=0x7ffff106a860, 
    to=0x7ffff1071cd8, in_dest=0, in_cond=0, unique_copy=0)
    at /export/gnu/import/git/gcc/gcc/combine.c:5164
#15 0x000000000110d1e3 in try_combine (i3=0x7ffff106c5a0,
i2=0x7ffff106c558, 
    i1=0x7ffff106c510, i0=0x0, new_direct_jump_p=0x7fffffffd974, 
    last_combined_insn=0x7ffff106c5a0)
---Type <return> to continue, or q <return> to quit---
    at /export/gnu/import/git/gcc/gcc/combine.c:3259
#16 0x00000000011083ad in combine_instructions (f=0x7ffff102ba40,
nregs=162)
    at /export/gnu/import/git/gcc/gcc/combine.c:1265
#17 0x00000000011284fd in rest_of_handle_combine ()
    at /export/gnu/import/git/gcc/gcc/combine.c:13948
#18 0x0000000000a1bfce in execute_one_pass (pass=0x19141c0
<pass_combine>)
    at /export/gnu/import/git/gcc/gcc/passes.c:2158
#19 0x0000000000a1c1b4 in execute_pass_list (pass=0x19141c0
<pass_combine>)
    at /export/gnu/import/git/gcc/gcc/passes.c:2213
#20 0x0000000000a1c1d5 in execute_pass_list (
    pass=0x190ea80 <pass_rest_of_compilation>)
    at /export/gnu/import/git/gcc/gcc/passes.c:2214
#21 0x000000000068a9f6 in expand_function (node=0x7ffff0f04750)
    at /export/gnu/import/git/gcc/gcc/cgraphunit.c:1610
#22 0x000000000068b0dd in expand_all_functions ()
    at /export/gnu/import/git/gcc/gcc/cgraphunit.c:1715
#23 0x000000000068bc5c in compile ()
    at /export/gnu/import/git/gcc/gcc/cgraphunit.c:2013
#24 0x000000000068bdbf in finalize_compilation_unit ()
    at /export/gnu/import/git/gcc/gcc/cgraphunit.c:2090
#25 0x00000000004bbf89 in c_write_global_declarations ()
    at /export/gnu/import/git/gcc/gcc/c/c-decl.c:10116
#26 0x0000000000b10237 in compile_file ()
---Type <return> to continue, or q <return> to quit---
    at /export/gnu/import/git/gcc/gcc/toplev.c:560
#27 0x0000000000b12119 in do_compile ()
    at /export/gnu/import/git/gcc/gcc/toplev.c:1863
#28 0x0000000000b12284 in toplev_main (argc=17, argv=0x7fffffffdde8)
    at /export/gnu/import/git/gcc/gcc/toplev.c:1939
#29 0x000000000123dcc8 in main (argc=17, argv=0x7fffffffdde8)
    at /export/gnu/import/git/gcc/gcc/main.c:36
(gdb) 

This patch calls convert_memory_address to convert address before
plus_constant.  OK for trunk?

Thanks.


H.J.
---
gcc/

2012-08-01  H.J. Lu  <hongjiu.lu@intel.com>

	PR rtl-optimization/54157
	* gcc/simplify-rtx.c (simplify_binary_operation_1): Call
	convert_memory_address to convert address before plus_constant.

gcc/testsuite/

2012-08-01  H.J. Lu  <hongjiu.lu@intel.com>

	PR rtl-optimization/54157
	* gcc.target/i386/pr54157.c: New file.

diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
index e1bd3cf..5d4ec89 100644
--- a/gcc/simplify-rtx.c
+++ b/gcc/simplify-rtx.c
@@ -1953,12 +1953,26 @@ simplify_binary_operation_1 (enum rtx_code code, enum machine_mode mode,
 	   || GET_CODE (op0) == SYMBOL_REF
 	   || GET_CODE (op0) == LABEL_REF)
 	  && CONST_INT_P (op1))
-	return plus_constant (mode, op0, INTVAL (op1));
+	{
+	  if (GET_CODE (op0) == CONST)
+	    return plus_constant (mode, op0, INTVAL (op1));
+	  else
+	    return plus_constant (mode,
+				  convert_memory_address (mode, op0),
+				  INTVAL (op1));
+	}
       else if ((GET_CODE (op1) == CONST
 		|| GET_CODE (op1) == SYMBOL_REF
 		|| GET_CODE (op1) == LABEL_REF)
 	       && CONST_INT_P (op0))
-	return plus_constant (mode, op1, INTVAL (op0));
+	{
+	  if (GET_CODE (op1) == CONST)
+	    return plus_constant (mode, op1, INTVAL (op0));
+	  else
+	    return plus_constant (mode,
+				  convert_memory_address (mode, op1),
+				  INTVAL (op0));
+	}
 
       /* See if this is something like X * C - X or vice versa or
 	 if the multiplication is written as a shift.  If so, we can
diff --git a/gcc/testsuite/gcc.target/i386/pr54157.c b/gcc/testsuite/gcc.target/i386/pr54157.c
new file mode 100644
index 0000000..b5c4528
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr54157.c
@@ -0,0 +1,21 @@
+/* { dg-do compile { target { ! { ia32 } } } } */
+/* { dg-options "-O2 -mx32 -maddress-mode=long -ftree-vectorize" } */
+
+struct s2{
+  int n[24 -1][24 -1][24 -1];
+};
+
+struct test2{
+  struct s2 e;
+};
+
+struct test2 tmp2[4];
+
+void main1 ()
+{
+  int i,j;
+
+  for (i = 0; i < 24 -4; i++)
+      for (j = 0; j < 24 -4; j++)
+          tmp2[2].e.n[1][i][j] = 8;
+}

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: PATCH: PR rtl-optimization/54157: [x32] -maddress-mode=long failures
  2012-08-01 18:41 PATCH: PR rtl-optimization/54157: [x32] -maddress-mode=long failures H.J. Lu
@ 2012-08-01 18:59 ` Richard Sandiford
  2012-08-01 19:14   ` H.J. Lu
  0 siblings, 1 reply; 12+ messages in thread
From: Richard Sandiford @ 2012-08-01 18:59 UTC (permalink / raw)
  To: H.J. Lu; +Cc: gcc-patches

"H.J. Lu" <hongjiu.lu@intel.com> writes:
> We have
>
> (gdb) r -fpreprocessed x.i -quiet -dumpbase x.i -mx32
> -maddress-mode=long -mtune=generic -march=x86-64 -auxbase x -O2 -version
> -ftree-vectorize -o x.s
> Starting program: /export/build/gnu/gcc-x32/build-x86_64-linux/gcc/cc1
> -fpreprocessed x.i -quiet -dumpbase x.i -mx32 -maddress-mode=long
> -mtune=generic -march=x86-64 -auxbase x -O2 -version -ftree-vectorize -o
> x.s
> GNU C (GCC) version 4.8.0 20120801 (experimental)
> (x86_64-unknown-linux-gnu)
> 	compiled by GNU C version 4.7.1 20120629 (Red Hat 4.7.1-1), GMP
> version 5.0.2, MPFR version 3.1.0, MPC version 0.9
> GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096
> GNU C (GCC) version 4.8.0 20120801 (experimental)
> (x86_64-unknown-linux-gnu)
> 	compiled by GNU C version 4.7.1 20120629 (Red Hat 4.7.1-1), GMP
> version 5.0.2, MPFR version 3.1.0, MPC version 0.9
> GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096
> Compiler executable checksum: 07a4e516c4e8fe4abfdafa83737d8f4a
>
> Breakpoint 1, fancy_abort (
>     file=0x130fe68 "/export/gnu/import/git/gcc/gcc/explow.c", line=88, 
>     function=0x131032e <__FUNCTION__.39220> "plus_constant")
>     at /export/gnu/import/git/gcc/gcc/diagnostic.c:1011
> 1011	  internal_error ("in %s, at %s:%d", function, trim_filename
> (file), line);
> (gdb) f 1
> #1  0x0000000000743e07 in plus_constant (mode=DImode, x=0x7ffff106a7e0, 
>     c=99452) at /export/gnu/import/git/gcc/gcc/explow.c:88
> 88	  gcc_assert (GET_MODE (x) == VOIDmode || GET_MODE (x) == mode);
> (gdb) f 2
> #2  0x0000000000adc4b1 in simplify_binary_operation_1 (code=PLUS,
> mode=DImode, 
>     op0=0x7ffff106a7e0, op1=0x7ffff1010e80, trueop0=0x7ffff106a7e0, 
>     trueop1=0x7ffff1010e80)
>     at /export/gnu/import/git/gcc/gcc/simplify-rtx.c:1956
> 1956		return plus_constant (mode, op0, INTVAL (op1));
> (gdb) call debug_rtx (op0)
> (symbol_ref:SI ("tmp2") <var_decl 0x7ffff0f06140 tmp2>)
> (gdb) call debug_rtx (op1)
> (const_int 99452 [0x1847c])
> (gdb) bt
> #0  fancy_abort (file=0x130fe68
> "/export/gnu/import/git/gcc/gcc/explow.c", 
>     line=88, function=0x131032e <__FUNCTION__.39220> "plus_constant")
>     at /export/gnu/import/git/gcc/gcc/diagnostic.c:1011
> #1  0x0000000000743e07 in plus_constant (mode=DImode, x=0x7ffff106a7e0, 
>     c=99452) at /export/gnu/import/git/gcc/gcc/explow.c:88
> #2  0x0000000000adc4b1 in simplify_binary_operation_1 (code=PLUS,
> mode=DImode, 
>     op0=0x7ffff106a7e0, op1=0x7ffff1010e80, trueop0=0x7ffff106a7e0, 
>     trueop1=0x7ffff1010e80)
>     at /export/gnu/import/git/gcc/gcc/simplify-rtx.c:1956
> #3  0x0000000000adc221 in simplify_binary_operation (code=PLUS,
> mode=DImode, 
>     op0=0x7ffff106a7e0, op1=0x7ffff1010e80)
>     at /export/gnu/import/git/gcc/gcc/simplify-rtx.c:1904

Things have already gone wrong by this frame: we have a DImode
addition of an SImode value, which isn't allowed.  Where does
that mismatch get introduced?

Richard

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: PATCH: PR rtl-optimization/54157: [x32] -maddress-mode=long failures
  2012-08-01 18:59 ` Richard Sandiford
@ 2012-08-01 19:14   ` H.J. Lu
  2012-08-02 18:21     ` H.J. Lu
  0 siblings, 1 reply; 12+ messages in thread
From: H.J. Lu @ 2012-08-01 19:14 UTC (permalink / raw)
  To: gcc-patches, rdsandiford

On Wed, Aug 1, 2012 at 11:58 AM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> "H.J. Lu" <hongjiu.lu@intel.com> writes:
>> We have
>>
>> (gdb) r -fpreprocessed x.i -quiet -dumpbase x.i -mx32
>> -maddress-mode=long -mtune=generic -march=x86-64 -auxbase x -O2 -version
>> -ftree-vectorize -o x.s
>> Starting program: /export/build/gnu/gcc-x32/build-x86_64-linux/gcc/cc1
>> -fpreprocessed x.i -quiet -dumpbase x.i -mx32 -maddress-mode=long
>> -mtune=generic -march=x86-64 -auxbase x -O2 -version -ftree-vectorize -o
>> x.s
>> GNU C (GCC) version 4.8.0 20120801 (experimental)
>> (x86_64-unknown-linux-gnu)
>>       compiled by GNU C version 4.7.1 20120629 (Red Hat 4.7.1-1), GMP
>> version 5.0.2, MPFR version 3.1.0, MPC version 0.9
>> GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096
>> GNU C (GCC) version 4.8.0 20120801 (experimental)
>> (x86_64-unknown-linux-gnu)
>>       compiled by GNU C version 4.7.1 20120629 (Red Hat 4.7.1-1), GMP
>> version 5.0.2, MPFR version 3.1.0, MPC version 0.9
>> GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096
>> Compiler executable checksum: 07a4e516c4e8fe4abfdafa83737d8f4a
>>
>> Breakpoint 1, fancy_abort (
>>     file=0x130fe68 "/export/gnu/import/git/gcc/gcc/explow.c", line=88,
>>     function=0x131032e <__FUNCTION__.39220> "plus_constant")
>>     at /export/gnu/import/git/gcc/gcc/diagnostic.c:1011
>> 1011    internal_error ("in %s, at %s:%d", function, trim_filename
>> (file), line);
>> (gdb) f 1
>> #1  0x0000000000743e07 in plus_constant (mode=DImode, x=0x7ffff106a7e0,
>>     c=99452) at /export/gnu/import/git/gcc/gcc/explow.c:88
>> 88      gcc_assert (GET_MODE (x) == VOIDmode || GET_MODE (x) == mode);
>> (gdb) f 2
>> #2  0x0000000000adc4b1 in simplify_binary_operation_1 (code=PLUS,
>> mode=DImode,
>>     op0=0x7ffff106a7e0, op1=0x7ffff1010e80, trueop0=0x7ffff106a7e0,
>>     trueop1=0x7ffff1010e80)
>>     at /export/gnu/import/git/gcc/gcc/simplify-rtx.c:1956
>> 1956          return plus_constant (mode, op0, INTVAL (op1));
>> (gdb) call debug_rtx (op0)
>> (symbol_ref:SI ("tmp2") <var_decl 0x7ffff0f06140 tmp2>)
>> (gdb) call debug_rtx (op1)
>> (const_int 99452 [0x1847c])
>> (gdb) bt
>> #0  fancy_abort (file=0x130fe68
>> "/export/gnu/import/git/gcc/gcc/explow.c",
>>     line=88, function=0x131032e <__FUNCTION__.39220> "plus_constant")
>>     at /export/gnu/import/git/gcc/gcc/diagnostic.c:1011
>> #1  0x0000000000743e07 in plus_constant (mode=DImode, x=0x7ffff106a7e0,
>>     c=99452) at /export/gnu/import/git/gcc/gcc/explow.c:88
>> #2  0x0000000000adc4b1 in simplify_binary_operation_1 (code=PLUS,
>> mode=DImode,
>>     op0=0x7ffff106a7e0, op1=0x7ffff1010e80, trueop0=0x7ffff106a7e0,
>>     trueop1=0x7ffff1010e80)
>>     at /export/gnu/import/git/gcc/gcc/simplify-rtx.c:1956
>> #3  0x0000000000adc221 in simplify_binary_operation (code=PLUS,
>> mode=DImode,
>>     op0=0x7ffff106a7e0, op1=0x7ffff1010e80)
>>     at /export/gnu/import/git/gcc/gcc/simplify-rtx.c:1904
>
> Things have already gone wrong by this frame: we have a DImode
> addition of an SImode value, which isn't allowed.  Where does
> that mismatch get introduced?
>

make_extraction in combine generates:

7474	      inner = force_to_mode (inner, wanted_inner_mode,
7475				     pos_rtx
7476				     || len + orig_pos >= HOST_BITS_PER_WIDE_INT
7477				     ? ~(unsigned HOST_WIDE_INT) 0
7478				     : ((((unsigned HOST_WIDE_INT) 1 << len) - 1)
(gdb) call debug_rtx (inner)
(plus:SI (reg:SI 109 [ D.1765 ])
    (const:SI (plus:SI (symbol_ref:SI ("tmp2") <var_decl 0x7ffff0f06140 tmp2>)
            (const_int 99452 [0x1847c]))))
(gdb) p wanted_inner_mode
$2 = DImode
(gdb)



-- 
H.J.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: PATCH: PR rtl-optimization/54157: [x32] -maddress-mode=long failures
  2012-08-01 19:14   ` H.J. Lu
@ 2012-08-02 18:21     ` H.J. Lu
  2012-08-05  7:47       ` Richard Sandiford
  0 siblings, 1 reply; 12+ messages in thread
From: H.J. Lu @ 2012-08-02 18:21 UTC (permalink / raw)
  To: gcc-patches, rdsandiford, Uros Bizjak

[-- Attachment #1: Type: text/plain, Size: 5529 bytes --]

On Wed, Aug 1, 2012 at 12:14 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Wed, Aug 1, 2012 at 11:58 AM, Richard Sandiford
> <rdsandiford@googlemail.com> wrote:
>> "H.J. Lu" <hongjiu.lu@intel.com> writes:
>>> We have
>>>
>>> (gdb) r -fpreprocessed x.i -quiet -dumpbase x.i -mx32
>>> -maddress-mode=long -mtune=generic -march=x86-64 -auxbase x -O2 -version
>>> -ftree-vectorize -o x.s
>>> Starting program: /export/build/gnu/gcc-x32/build-x86_64-linux/gcc/cc1
>>> -fpreprocessed x.i -quiet -dumpbase x.i -mx32 -maddress-mode=long
>>> -mtune=generic -march=x86-64 -auxbase x -O2 -version -ftree-vectorize -o
>>> x.s
>>> GNU C (GCC) version 4.8.0 20120801 (experimental)
>>> (x86_64-unknown-linux-gnu)
>>>       compiled by GNU C version 4.7.1 20120629 (Red Hat 4.7.1-1), GMP
>>> version 5.0.2, MPFR version 3.1.0, MPC version 0.9
>>> GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096
>>> GNU C (GCC) version 4.8.0 20120801 (experimental)
>>> (x86_64-unknown-linux-gnu)
>>>       compiled by GNU C version 4.7.1 20120629 (Red Hat 4.7.1-1), GMP
>>> version 5.0.2, MPFR version 3.1.0, MPC version 0.9
>>> GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096
>>> Compiler executable checksum: 07a4e516c4e8fe4abfdafa83737d8f4a
>>>
>>> Breakpoint 1, fancy_abort (
>>>     file=0x130fe68 "/export/gnu/import/git/gcc/gcc/explow.c", line=88,
>>>     function=0x131032e <__FUNCTION__.39220> "plus_constant")
>>>     at /export/gnu/import/git/gcc/gcc/diagnostic.c:1011
>>> 1011    internal_error ("in %s, at %s:%d", function, trim_filename
>>> (file), line);
>>> (gdb) f 1
>>> #1  0x0000000000743e07 in plus_constant (mode=DImode, x=0x7ffff106a7e0,
>>>     c=99452) at /export/gnu/import/git/gcc/gcc/explow.c:88
>>> 88      gcc_assert (GET_MODE (x) == VOIDmode || GET_MODE (x) == mode);
>>> (gdb) f 2
>>> #2  0x0000000000adc4b1 in simplify_binary_operation_1 (code=PLUS,
>>> mode=DImode,
>>>     op0=0x7ffff106a7e0, op1=0x7ffff1010e80, trueop0=0x7ffff106a7e0,
>>>     trueop1=0x7ffff1010e80)
>>>     at /export/gnu/import/git/gcc/gcc/simplify-rtx.c:1956
>>> 1956          return plus_constant (mode, op0, INTVAL (op1));
>>> (gdb) call debug_rtx (op0)
>>> (symbol_ref:SI ("tmp2") <var_decl 0x7ffff0f06140 tmp2>)
>>> (gdb) call debug_rtx (op1)
>>> (const_int 99452 [0x1847c])
>>> (gdb) bt
>>> #0  fancy_abort (file=0x130fe68
>>> "/export/gnu/import/git/gcc/gcc/explow.c",
>>>     line=88, function=0x131032e <__FUNCTION__.39220> "plus_constant")
>>>     at /export/gnu/import/git/gcc/gcc/diagnostic.c:1011
>>> #1  0x0000000000743e07 in plus_constant (mode=DImode, x=0x7ffff106a7e0,
>>>     c=99452) at /export/gnu/import/git/gcc/gcc/explow.c:88
>>> #2  0x0000000000adc4b1 in simplify_binary_operation_1 (code=PLUS,
>>> mode=DImode,
>>>     op0=0x7ffff106a7e0, op1=0x7ffff1010e80, trueop0=0x7ffff106a7e0,
>>>     trueop1=0x7ffff1010e80)
>>>     at /export/gnu/import/git/gcc/gcc/simplify-rtx.c:1956
>>> #3  0x0000000000adc221 in simplify_binary_operation (code=PLUS,
>>> mode=DImode,
>>>     op0=0x7ffff106a7e0, op1=0x7ffff1010e80)
>>>     at /export/gnu/import/git/gcc/gcc/simplify-rtx.c:1904
>>
>> Things have already gone wrong by this frame: we have a DImode
>> addition of an SImode value, which isn't allowed.  Where does
>> that mismatch get introduced?
>>
>
> make_extraction in combine generates:
>
> 7474          inner = force_to_mode (inner, wanted_inner_mode,
> 7475                                 pos_rtx
> 7476                                 || len + orig_pos >= HOST_BITS_PER_WIDE_INT
> 7477                                 ? ~(unsigned HOST_WIDE_INT) 0
> 7478                                 : ((((unsigned HOST_WIDE_INT) 1 << len) - 1)
> (gdb) call debug_rtx (inner)
> (plus:SI (reg:SI 109 [ D.1765 ])
>     (const:SI (plus:SI (symbol_ref:SI ("tmp2") <var_decl 0x7ffff0f06140 tmp2>)
>             (const_int 99452 [0x1847c]))))
> (gdb) p wanted_inner_mode
> $2 = DImode
> (gdb)
>

i386,md has

(define_expand "extzv"
  [(set (match_operand:SI 0 "register_operand")
        (zero_extract:SI (match_operand 1 "ext_register_operand")
                         (match_operand:SI 2 "const8_operand")
                         (match_operand:SI 3 "const8_operand")))]
  ""

and mode_for_extraction picks word_mode for operand 1 since
its mode is VOIDmode.  This patch changes mode_for_extraction
to return the mode of operand 1 if the pattern accepts any mode.
I added *jcc_btsi_mask_2 since combine now tries a different
pattern, which leads to test failures on gcc.target/i386/bt-mask-1.c
and  gcc.target/i386/bt-mask-2.  I didn't update *jcc_btsi_mask_1
instead since I am not sure if it is used elsewhere.  Tested on
Linux/x86-64 and Linux/x32.  OK for trunk?

Thanks.

-- 
H.J.
----
gcc/

2012-08-02  H.J. Lu  <hongjiu.lu@intel.com>

	PR rtl-optimization/54157
	* combine.c (make_extraction): Pass inner_mode to mode_for_extraction
	for operand 1, otherwise pass VOIDmode.
	(simplify_comparison): Pass VOIDmode to mode_for_extraction.

	* expmed.c (mode_for_extraction): Add a machine_mode argument
	and use it instead of word_mode if it isn't VOIDmode.
	(store_bit_field_1): Pass VOIDmode to mode_for_extraction.
	(store_bit_field): Likewise.

	* expr.h (mode_for_extraction): Add a machine_mode argument.

	* config/i386/i386.md (*jcc_btsi_mask_2): New insn_and_split
	pattern.

	* recog.c (simplify_while_replacing): Pass is_mode to
	mode_for_extraction.

gcc/testsuite/

2012-08-02  H.J. Lu  <hongjiu.lu@intel.com>

	PR rtl-optimization/54157
	* gcc.target/i386/pr54157.c: New file.

[-- Attachment #2: gcc-pr54157.patch --]
[-- Type: application/octet-stream, Size: 8565 bytes --]

gcc/

2012-08-02  H.J. Lu  <hongjiu.lu@intel.com>

	PR rtl-optimization/54157
	* combine.c (make_extraction): Pass inner_mode to mode_for_extraction
	for operand 1, otherwise pass VOIDmode.
	(simplify_comparison): Pass VOIDmode to mode_for_extraction.

	* expmed.c (mode_for_extraction): Add a machine_mode argument
	and use it instead of word_mode if it isn't VOIDmode.
	(store_bit_field_1): Pass VOIDmode to mode_for_extraction.
	(store_bit_field): Likewise.

	* expr.h (mode_for_extraction): Add a machine_mode argument.

	* config/i386/i386.md (*jcc_btsi_mask_2): New insn_and_split
	pattern.

	* recog.c (simplify_while_replacing): Pass is_mode to
	mode_for_extraction.

gcc/testsuite/

2012-08-02  H.J. Lu  <hongjiu.lu@intel.com>

	PR rtl-optimization/54157
	* gcc.target/i386/pr54157.c: New file.

diff --git a/gcc/combine.c b/gcc/combine.c
index a07c046..035aeb9 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -7332,27 +7332,30 @@ make_extraction (enum machine_mode mode, rtx inner, HOST_WIDE_INT pos,
 
   /* Get the mode to use should INNER not be a MEM, the mode for the position,
      and the mode for the result.  */
-  if (in_dest && mode_for_extraction (EP_insv, -1) != MAX_MACHINE_MODE)
+  if (in_dest
+      && mode_for_extraction (EP_insv, -1, VOIDmode) != MAX_MACHINE_MODE)
     {
-      wanted_inner_reg_mode = mode_for_extraction (EP_insv, 0);
-      pos_mode = mode_for_extraction (EP_insv, 2);
-      extraction_mode = mode_for_extraction (EP_insv, 3);
+      wanted_inner_reg_mode = mode_for_extraction (EP_insv, 0, VOIDmode);
+      pos_mode = mode_for_extraction (EP_insv, 2, VOIDmode);
+      extraction_mode = mode_for_extraction (EP_insv, 3, VOIDmode);
     }
 
   if (! in_dest && unsignedp
-      && mode_for_extraction (EP_extzv, -1) != MAX_MACHINE_MODE)
+      && mode_for_extraction (EP_extzv, -1, VOIDmode) != MAX_MACHINE_MODE)
     {
-      wanted_inner_reg_mode = mode_for_extraction (EP_extzv, 1);
-      pos_mode = mode_for_extraction (EP_extzv, 3);
-      extraction_mode = mode_for_extraction (EP_extzv, 0);
+      wanted_inner_reg_mode = mode_for_extraction (EP_extzv, 1,
+						   inner_mode);
+      pos_mode = mode_for_extraction (EP_extzv, 3, VOIDmode);
+      extraction_mode = mode_for_extraction (EP_extzv, 0, VOIDmode);
     }
 
   if (! in_dest && ! unsignedp
-      && mode_for_extraction (EP_extv, -1) != MAX_MACHINE_MODE)
+      && mode_for_extraction (EP_extv, -1, VOIDmode) != MAX_MACHINE_MODE)
     {
-      wanted_inner_reg_mode = mode_for_extraction (EP_extv, 1);
-      pos_mode = mode_for_extraction (EP_extv, 3);
-      extraction_mode = mode_for_extraction (EP_extv, 0);
+      wanted_inner_reg_mode = mode_for_extraction (EP_extv, 1,
+						   inner_mode);
+      pos_mode = mode_for_extraction (EP_extv, 3, VOIDmode);
+      extraction_mode = mode_for_extraction (EP_extv, 0, VOIDmode);
     }
 
   /* Never narrow an object, since that might not be safe.  */
@@ -11294,7 +11297,7 @@ simplify_comparison (enum rtx_code code, rtx *pop0, rtx *pop1)
 	      if (BITS_BIG_ENDIAN)
 		{
 		  enum machine_mode new_mode
-		    = mode_for_extraction (EP_extzv, 1);
+		    = mode_for_extraction (EP_extzv, 1, VOIDmode);
 		  if (new_mode == MAX_MACHINE_MODE)
 		    i = BITS_PER_WORD - 1 - i;
 		  else
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index ace3b6e..6835d31 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -11065,6 +11065,39 @@
 		      (pc)))]
   "PUT_CODE (operands[0], reverse_condition (GET_CODE (operands[0])));")
 
+(define_insn_and_split "*jcc_btsi_mask_2"
+  [(set (pc)
+  	(if_then_else
+	  (match_operator 0 "bt_comparison_operator"
+	    [(zero_extract:SI
+	       (match_operand:SI 1 "register_operand" "r")
+	       (const_int 1)
+	       (zero_extend:SI
+		 (subreg:QI
+		   (and:SI
+		     (match_operand:SI 2 "register_operand" "r")
+		     (match_operand:SI 3 "const_int_operand" "n")) 0)))
+	     (const_int 0)])
+	  (label_ref (match_operand 4))
+	  (pc)))
+   (clobber (reg:CC FLAGS_REG))]
+  "(TARGET_USE_BT || optimize_function_for_size_p (cfun))
+   && (INTVAL (operands[3]) & 0x1f) == 0x1f"
+  "#"
+  "&& 1"
+  [(set (reg:CCC FLAGS_REG)
+	(compare:CCC
+	  (zero_extract:SI
+	    (match_dup 1)
+	    (const_int 1)
+	    (match_dup 2))
+	  (const_int 0)))
+   (set (pc)
+	(if_then_else (match_op_dup 0 [(reg:CCC FLAGS_REG) (const_int 0)])
+		      (label_ref (match_dup 4))
+		      (pc)))]
+  "PUT_CODE (operands[0], reverse_condition (GET_CODE (operands[0])));")
+
 ;; Define combination compare-and-branch fp compare instructions to help
 ;; combine.
 
diff --git a/gcc/expmed.c b/gcc/expmed.c
index 1fe0034..367f94d 100644
--- a/gcc/expmed.c
+++ b/gcc/expmed.c
@@ -338,9 +338,12 @@ negate_rtx (enum machine_mode mode, rtx x)
 /* Report on the availability of insv/extv/extzv and the desired mode
    of each of their operands.  Returns MAX_MACHINE_MODE if HAVE_foo
    is false; else the mode of the specified operand.  If OPNO is -1,
-   all the caller cares about is whether the insn is available.  */
+   all the caller cares about is whether the insn is available.  Return
+   MODE instead of word_mode if it isn't VOIDmode.  */
+
 enum machine_mode
-mode_for_extraction (enum extraction_pattern pattern, int opno)
+mode_for_extraction (enum extraction_pattern pattern, int opno,
+		     enum machine_mode mode)
 {
   const struct insn_data_d *data;
 
@@ -380,7 +383,7 @@ mode_for_extraction (enum extraction_pattern pattern, int opno)
   /* Everyone who uses this function used to follow it with
      if (result == VOIDmode) result = word_mode; */
   if (data->operand[opno].mode == VOIDmode)
-    return word_mode;
+    return mode != VOIDmode ? mode : word_mode;
   return data->operand[opno].mode;
 }
 \f
@@ -406,7 +409,7 @@ store_bit_field_1 (rtx str_rtx, unsigned HOST_WIDE_INT bitsize,
   int byte_offset;
   rtx orig_value;
 
-  enum machine_mode op_mode = mode_for_extraction (EP_insv, 3);
+  enum machine_mode op_mode = mode_for_extraction (EP_insv, 3, VOIDmode);
 
   while (GET_CODE (op0) == SUBREG)
     {
@@ -894,7 +897,7 @@ store_bit_field (rtx str_rtx, unsigned HOST_WIDE_INT bitsize,
       enum machine_mode op_mode;
       unsigned HOST_WIDE_INT offset;
 
-      op_mode = mode_for_extraction (EP_insv, 3);
+      op_mode = mode_for_extraction (EP_insv, 3, VOIDmode);
       if (op_mode == MAX_MACHINE_MODE)
 	op_mode = VOIDmode;
 
@@ -1592,7 +1595,8 @@ extract_bit_field_1 (rtx str_rtx, unsigned HOST_WIDE_INT bitsize,
     }
 
   /* Now OFFSET is nonzero only for memory operands.  */
-  ext_mode = mode_for_extraction (unsignedp ? EP_extzv : EP_extv, 0);
+  ext_mode = mode_for_extraction (unsignedp ? EP_extzv : EP_extv, 0,
+				  VOIDmode);
   if (ext_mode != MAX_MACHINE_MODE
       && bitsize > 0
       && GET_MODE_BITSIZE (ext_mode) >= bitsize
diff --git a/gcc/expr.h b/gcc/expr.h
index 68cdb8d..588299c 100644
--- a/gcc/expr.h
+++ b/gcc/expr.h
@@ -687,7 +687,7 @@ extern rtx hard_libcall_value (enum machine_mode, rtx);
 
 enum extraction_pattern { EP_insv, EP_extv, EP_extzv };
 extern enum machine_mode
-mode_for_extraction (enum extraction_pattern, int);
+mode_for_extraction (enum extraction_pattern, int, enum machine_mode);
 
 extern void store_bit_field (rtx, unsigned HOST_WIDE_INT,
 			     unsigned HOST_WIDE_INT,
diff --git a/gcc/recog.c b/gcc/recog.c
index a05e8c6..2d1e4db 100644
--- a/gcc/recog.c
+++ b/gcc/recog.c
@@ -639,14 +639,14 @@ simplify_while_replacing (rtx *loc, rtx to, rtx object,
 	  if (GET_CODE (x) == ZERO_EXTRACT)
 	    {
 	      enum machine_mode new_mode
-		= mode_for_extraction (EP_extzv, 1);
+		= mode_for_extraction (EP_extzv, 1, is_mode);
 	      if (new_mode != MAX_MACHINE_MODE)
 		wanted_mode = new_mode;
 	    }
 	  else if (GET_CODE (x) == SIGN_EXTRACT)
 	    {
 	      enum machine_mode new_mode
-		= mode_for_extraction (EP_extv, 1);
+		= mode_for_extraction (EP_extv, 1, is_mode);
 	      if (new_mode != MAX_MACHINE_MODE)
 		wanted_mode = new_mode;
 	    }
diff --git a/gcc/testsuite/gcc.target/i386/pr54157.c b/gcc/testsuite/gcc.target/i386/pr54157.c
new file mode 100644
index 0000000..b5c4528
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr54157.c
@@ -0,0 +1,21 @@
+/* { dg-do compile { target { ! { ia32 } } } } */
+/* { dg-options "-O2 -mx32 -maddress-mode=long -ftree-vectorize" } */
+
+struct s2{
+  int n[24 -1][24 -1][24 -1];
+};
+
+struct test2{
+  struct s2 e;
+};
+
+struct test2 tmp2[4];
+
+void main1 ()
+{
+  int i,j;
+
+  for (i = 0; i < 24 -4; i++)
+      for (j = 0; j < 24 -4; j++)
+          tmp2[2].e.n[1][i][j] = 8;
+}

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: PATCH: PR rtl-optimization/54157: [x32] -maddress-mode=long failures
  2012-08-02 18:21     ` H.J. Lu
@ 2012-08-05  7:47       ` Richard Sandiford
  2012-08-07 19:28         ` H.J. Lu
  0 siblings, 1 reply; 12+ messages in thread
From: Richard Sandiford @ 2012-08-05  7:47 UTC (permalink / raw)
  To: H.J. Lu; +Cc: gcc-patches, Uros Bizjak

For the record, I can't approve this, but...

"H.J. Lu" <hjl.tools@gmail.com> writes:
> i386,md has
>
> (define_expand "extzv"
>   [(set (match_operand:SI 0 "register_operand")
>         (zero_extract:SI (match_operand 1 "ext_register_operand")
>                          (match_operand:SI 2 "const8_operand")
>                          (match_operand:SI 3 "const8_operand")))]
>   ""
>
> and mode_for_extraction picks word_mode for operand 1 since
> its mode is VOIDmode.  This patch changes mode_for_extraction
> to return the mode of operand 1 if the pattern accepts any mode.
> I added *jcc_btsi_mask_2 since combine now tries a different
> pattern, which leads to test failures on gcc.target/i386/bt-mask-1.c
> and  gcc.target/i386/bt-mask-2.  I didn't update *jcc_btsi_mask_1
> instead since I am not sure if it is used elsewhere.  Tested on
> Linux/x86-64 and Linux/x32.  OK for trunk?

the mode of the extraction operand is defined to be word_mode
for registers (see md.texi), so that at least would need to
be updated.  But I'm not convinced that the wanted_inner_mode here:

   if (! in_dest && unsignedp
-      && mode_for_extraction (EP_extzv, -1) != MAX_MACHINE_MODE)
+      && mode_for_extraction (EP_extzv, -1, VOIDmode) != MAX_MACHINE_MODE)
     {
-      wanted_inner_reg_mode = mode_for_extraction (EP_extzv, 1);
-      pos_mode = mode_for_extraction (EP_extzv, 3);
-      extraction_mode = mode_for_extraction (EP_extzv, 0);
+      wanted_inner_reg_mode = mode_for_extraction (EP_extzv, 1,
+						   inner_mode);
+      pos_mode = mode_for_extraction (EP_extzv, 3, VOIDmode);
+      extraction_mode = mode_for_extraction (EP_extzv, 0, VOIDmode);
     }

is right.  inner_mode is the mode of the thing we're extracting,
which doesn't ncessarily have anything to do with what the ext*
patterns support.

FWIW, in reply to your force_to_mode message, gen_lowpart_for_combine
looks a bit odd:

  if (omode == imode)
    return x;

  /* Return identity if this is a CONST or symbolic reference.  */
  if (omode == Pmode
      && (GET_CODE (x) == CONST
	  || GET_CODE (x) == SYMBOL_REF
	  || GET_CODE (x) == LABEL_REF))
    return x;

So if we know the modes are different, we nevertheless return the
original mode for CONST, SYMBOL_REF or LABEL_REF.  Surely the caller
isn't going to be expecting that?  If we're not prepared to change
the mode to the one that the caller asked for then I think we should
goto fail instead.

I don't know if you're hitting that or not.

Richard

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: PATCH: PR rtl-optimization/54157: [x32] -maddress-mode=long failures
  2012-08-05  7:47       ` Richard Sandiford
@ 2012-08-07 19:28         ` H.J. Lu
  2012-08-08  8:09           ` Richard Sandiford
  0 siblings, 1 reply; 12+ messages in thread
From: H.J. Lu @ 2012-08-07 19:28 UTC (permalink / raw)
  To: gcc-patches, Uros Bizjak, rdsandiford

On Sun, Aug 5, 2012 at 12:47 AM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> For the record, I can't approve this, but...
>
> "H.J. Lu" <hjl.tools@gmail.com> writes:
>> i386,md has
>>
>> (define_expand "extzv"
>>   [(set (match_operand:SI 0 "register_operand")
>>         (zero_extract:SI (match_operand 1 "ext_register_operand")
>>                          (match_operand:SI 2 "const8_operand")
>>                          (match_operand:SI 3 "const8_operand")))]
>>   ""
>>
>> and mode_for_extraction picks word_mode for operand 1 since
>> its mode is VOIDmode.  This patch changes mode_for_extraction
>> to return the mode of operand 1 if the pattern accepts any mode.
>> I added *jcc_btsi_mask_2 since combine now tries a different
>> pattern, which leads to test failures on gcc.target/i386/bt-mask-1.c
>> and  gcc.target/i386/bt-mask-2.  I didn't update *jcc_btsi_mask_1
>> instead since I am not sure if it is used elsewhere.  Tested on
>> Linux/x86-64 and Linux/x32.  OK for trunk?
>
> the mode of the extraction operand is defined to be word_mode
> for registers (see md.texi), so that at least would need to
> be updated.  But I'm not convinced that the wanted_inner_mode here:
>
>    if (! in_dest && unsignedp
> -      && mode_for_extraction (EP_extzv, -1) != MAX_MACHINE_MODE)
> +      && mode_for_extraction (EP_extzv, -1, VOIDmode) != MAX_MACHINE_MODE)
>      {
> -      wanted_inner_reg_mode = mode_for_extraction (EP_extzv, 1);
> -      pos_mode = mode_for_extraction (EP_extzv, 3);
> -      extraction_mode = mode_for_extraction (EP_extzv, 0);
> +      wanted_inner_reg_mode = mode_for_extraction (EP_extzv, 1,
> +                                                  inner_mode);
> +      pos_mode = mode_for_extraction (EP_extzv, 3, VOIDmode);
> +      extraction_mode = mode_for_extraction (EP_extzv, 0, VOIDmode);
>      }
>
> is right.  inner_mode is the mode of the thing we're extracting,
> which doesn't ncessarily have anything to do with what the ext*
> patterns support.
>
> FWIW, in reply to your force_to_mode message, gen_lowpart_for_combine
> looks a bit odd:
>
>   if (omode == imode)
>     return x;
>
>   /* Return identity if this is a CONST or symbolic reference.  */
>   if (omode == Pmode
>       && (GET_CODE (x) == CONST
>           || GET_CODE (x) == SYMBOL_REF
>           || GET_CODE (x) == LABEL_REF))
>     return x;
>
> So if we know the modes are different, we nevertheless return the
> original mode for CONST, SYMBOL_REF or LABEL_REF.  Surely the caller
> isn't going to be expecting that?  If we're not prepared to change
> the mode to the one that the caller asked for then I think we should
> goto fail instead.
>
> I don't know if you're hitting that or not.
>
> Richard

Yes, I did

Breakpoint 5, gen_lowpart_for_combine (omode=DImode, x=0x7ffff1b03440)
    at /export/gnu/import/git/gcc-misc/gcc/combine.c:10778
10778	{
(const:SI (plus:SI (symbol_ref:SI ("tmp2") <var_decl 0x7ffff1997140 tmp2>)
        (const_int 99452 [0x1847c])))
(gdb) c
Continuing.

Breakpoint 5, gen_lowpart_for_combine (omode=DImode, x=0x7ffff1b03440)
    at /export/gnu/import/git/gcc-misc/gcc/combine.c:10778
10778	{
(const:SI (plus:SI (symbol_ref:SI ("tmp2") <var_decl 0x7ffff1997140 tmp2>)
        (const_int 99452 [0x1847c])))
(gdb) bt
#0  gen_lowpart_for_combine (omode=DImode, x=0x7ffff1b03440)
    at /export/gnu/import/git/gcc-misc/gcc/combine.c:10778
#1  0x0000000000c83805 in gen_lowpart_or_truncate (x=0x7ffff1b03440,
    mode=DImode) at /export/gnu/import/git/gcc-misc/gcc/combine.c:8110
#2  force_to_mode (x=0x7ffff1b02cd8, mode=DImode, mask=<optimized out>,
    just_select=<optimized out>)
    at /export/gnu/import/git/gcc-misc/gcc/combine.c:8389
#3  0x0000000000c8458a in make_extraction (mode=SImode, inner=0x7ffff1b02cd8,
    pos=2, pos_rtx=<optimized out>, len=2, unsignedp=1,
    in_dest=<optimized out>, in_compare=0)
    at /export/gnu/import/git/gcc-misc/gcc/combine.c:7480
#4  0x0000000000c86f9c in make_compound_operation (x=x@entry=0x7ffff1b02d68,
    in_code=in_code@entry=SET)
    at /export/gnu/import/git/gcc-misc/gcc/combine.c:7863
#5  0x0000000000c89924 in simplify_set (x=0x7ffff1af7c78)
    at /export/gnu/import/git/gcc-misc/gcc/combine.c:6539
#6  combine_simplify_rtx (x=0x7ffff1af7c78, op0_mode=VOIDmode,
    in_dest=in_dest@entry=0, in_cond=in_cond@entry=0)
    at /export/gnu/import/git/gcc-misc/gcc/combine.c:5971
#7  0x0000000000c8bd1b in subst (x=<optimized out>, from=<optimized out>,
    to=<optimized out>, in_dest=0, in_cond=0, unique_copy=0)
    at /export/gnu/import/git/gcc-misc/gcc/combine.c:5301
#8  0x0000000000c8b8ab in subst (x=0x7ffff1aea870, from=0x7ffff1afc880,
---Type <return> to continue, or q <return> to quit---
    to=0x7ffff1b02cd8, in_dest=0, in_cond=0, unique_copy=0)
    at /export/gnu/import/git/gcc-misc/gcc/combine.c:5165
#9  0x0000000000c8f875 in try_combine (i3=i3@entry=0x7ffff1afe5a0,
    i2=i2@entry=0x7ffff1afe558, i1=<optimized out>, i0=<optimized out>,
    i0@entry=0x0, new_direct_jump_p=new_direct_jump_p@entry=0x7fffffffdb64,
    last_combined_insn=last_combined_insn@entry=0x7ffff1afe5a0)
    at /export/gnu/import/git/gcc-misc/gcc/combine.c:3260
#10 0x0000000000c94673 in combine_instructions (nregs=<optimized out>,
    f=<optimized out>) at /export/gnu/import/git/gcc-misc/gcc/combine.c:1265
#11 rest_of_handle_combine ()
    at /export/gnu/import/git/gcc-misc/gcc/combine.c:13948
#12 0x0000000000824755 in execute_one_pass (
    pass=pass@entry=0x135aae0 <pass_combine>)
    at /export/gnu/import/git/gcc-misc/gcc/passes.c:2158
#13 0x0000000000824b15 in execute_pass_list (pass=0x135aae0 <pass_combine>)
    at /export/gnu/import/git/gcc-misc/gcc/passes.c:2213
#14 0x0000000000824b27 in execute_pass_list (
    pass=0x1355d20 <pass_rest_of_compilation>)
    at /export/gnu/import/git/gcc-misc/gcc/passes.c:2214
#15 0x0000000000610fb8 in expand_function (node=0x7ffff1995750)
    at /export/gnu/import/git/gcc-misc/gcc/cgraphunit.c:1610
#16 0x0000000000612df4 in expand_all_functions ()
    at /export/gnu/import/git/gcc-misc/gcc/cgraphunit.c:1715
---Type <return> to continue, or q <return> to quit---
#17 compile () at /export/gnu/import/git/gcc-misc/gcc/cgraphunit.c:2013
#18 0x00000000006133b5 in finalize_compilation_unit ()
    at /export/gnu/import/git/gcc-misc/gcc/cgraphunit.c:2090
#19 0x0000000000506b90 in c_write_global_declarations ()
    at /export/gnu/import/git/gcc-misc/gcc/c/c-decl.c:10116
#20 0x00000000008c5325 in compile_file ()
    at /export/gnu/import/git/gcc-misc/gcc/toplev.c:560
#21 0x00000000008c6eb8 in do_compile ()
    at /export/gnu/import/git/gcc-misc/gcc/toplev.c:1863
#22 toplev_main (argc=17, argv=0x7fffffffdde8)
    at /export/gnu/import/git/gcc-misc/gcc/toplev.c:1939
#23 0x0000003c88221735 in __libc_start_main () from /lib64/libc.so.6
#24 0x00000000004e9f41 in _start ()

This patch:

diff --git a/gcc/combine.c b/gcc/combine.c
index a07c046..b9a3589 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -10784,12 +10784,30 @@ gen_lowpart_for_combine (enum machine_mode omode, rtx
x)
   if (omode == imode)
     return x;

-  /* Return identity if this is a CONST or symbolic reference.  */
-  if (omode == Pmode
-      && (GET_CODE (x) == CONST
-	  || GET_CODE (x) == SYMBOL_REF
-	  || GET_CODE (x) == LABEL_REF))
-    return x;
+  if (omode == Pmode)
+    {
+      /* Return identity if this is a symbolic reference.  */
+      if (GET_CODE (x) == SYMBOL_REF || GET_CODE (x) == LABEL_REF)
+	return x;
+
+      /* Return identity for CONST unless this is a PLUS of 2 constant
+	 operands.  */
+      if (GET_CODE (x) == CONST)
+	{
+	  rtx y = XEXP (x, 0);
+	  if (GET_CODE (y) == PLUS
+	      && ((CONST_INT_P (XEXP (y, 1))
+		   && (GET_CODE (XEXP (y, 0)) == CONST
+		       || GET_CODE (XEXP (y, 0)) == SYMBOL_REF
+		       || GET_CODE (XEXP (y, 0)) == LABEL_REF))
+		  || (CONST_INT_P (XEXP (y, 1))
+		      && (GET_CODE (XEXP (y, 0)) == CONST
+			  || GET_CODE (XEXP (y, 0)) == SYMBOL_REF
+			  || GET_CODE (XEXP (y, 0)) == LABEL_REF))))
+	    goto fail;
+	  return x;
+	}
+    }

   /* We can only support MODE being wider than a word if X is a
      constant integer or has a mode the same size.  */

works for the testcase.


-- 
H.J.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: PATCH: PR rtl-optimization/54157: [x32] -maddress-mode=long failures
  2012-08-07 19:28         ` H.J. Lu
@ 2012-08-08  8:09           ` Richard Sandiford
  2012-08-08 13:40             ` H.J. Lu
  0 siblings, 1 reply; 12+ messages in thread
From: Richard Sandiford @ 2012-08-08  8:09 UTC (permalink / raw)
  To: H.J. Lu; +Cc: gcc-patches, Uros Bizjak

"H.J. Lu" <hjl.tools@gmail.com> writes:
> diff --git a/gcc/combine.c b/gcc/combine.c
> index a07c046..b9a3589 100644
> --- a/gcc/combine.c
> +++ b/gcc/combine.c
> @@ -10784,12 +10784,30 @@ gen_lowpart_for_combine (enum machine_mode omode, rtx
> x)
>    if (omode == imode)
>      return x;
>
> -  /* Return identity if this is a CONST or symbolic reference.  */
> -  if (omode == Pmode
> -      && (GET_CODE (x) == CONST
> -	  || GET_CODE (x) == SYMBOL_REF
> -	  || GET_CODE (x) == LABEL_REF))
> -    return x;
> +  if (omode == Pmode)
> +    {
> +      /* Return identity if this is a symbolic reference.  */
> +      if (GET_CODE (x) == SYMBOL_REF || GET_CODE (x) == LABEL_REF)
> +	return x;
> +
> +      /* Return identity for CONST unless this is a PLUS of 2 constant
> +	 operands.  */
> +      if (GET_CODE (x) == CONST)
> +	{
> +	  rtx y = XEXP (x, 0);
> +	  if (GET_CODE (y) == PLUS
> +	      && ((CONST_INT_P (XEXP (y, 1))
> +		   && (GET_CODE (XEXP (y, 0)) == CONST
> +		       || GET_CODE (XEXP (y, 0)) == SYMBOL_REF
> +		       || GET_CODE (XEXP (y, 0)) == LABEL_REF))
> +		  || (CONST_INT_P (XEXP (y, 1))
> +		      && (GET_CODE (XEXP (y, 0)) == CONST
> +			  || GET_CODE (XEXP (y, 0)) == SYMBOL_REF
> +			  || GET_CODE (XEXP (y, 0)) == LABEL_REF))))
> +	    goto fail;
> +	  return x;
> +	}
> +    }
>
>    /* We can only support MODE being wider than a word if X is a
>       constant integer or has a mode the same size.  */
>
> works for the testcase.

My point was that the "return x" is always wrong.  Whenever we return x
here, we know we're returning something in a different mode from the one
that the caller wanted.  Returning a Pmode LABEL_REF might not trigger
that plus_constant assert, but it's still wrong.

It looks like this came from the mips-rewrite branch:

2003-03-13  Richard Sandiford  <rsandifo@redhat.com>

        * combine.c (gen_lowpart_for_combine): Treat the lowpart Pmode of
        a CONST as identity.  Check the return value of gen_lowpart_common.

so I can categorically confirm that the person who wrote it didn't
know what they were doing.  It also means that this case was motivated
by an experiment to make Pmode == DImode for n32, which we ended up
discarding because it produced worse code.

If this case really is important, we might consider using
convert_memory_address (Pmode, x) instead.  I'm not sure whether
that would be right for targets with address spaces though, because
we don't know which address space is associated with the address.
Hopefully someone who knows address spaces can comment.

If it is correct, then it should probably go in gen_lowpart_common
rather than gen_lowpart_for_combine.

But given how few people have hit this, it doesn't look like a
particularly important attempted optimisation.  I'll pre-approve
a patch that undoes my mistake and simply removes:

  /* Return identity if this is a CONST or symbolic reference.  */
  if (omode == Pmode
      && (GET_CODE (x) == CONST
	  || GET_CODE (x) == SYMBOL_REF
	  || GET_CODE (x) == LABEL_REF))
    return x;

Richard

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: PATCH: PR rtl-optimization/54157: [x32] -maddress-mode=long failures
  2012-08-08  8:09           ` Richard Sandiford
@ 2012-08-08 13:40             ` H.J. Lu
  2012-08-08 13:43               ` Uros Bizjak
  0 siblings, 1 reply; 12+ messages in thread
From: H.J. Lu @ 2012-08-08 13:40 UTC (permalink / raw)
  To: H.J. Lu, gcc-patches, Uros Bizjak, rdsandiford

On Wed, Aug 8, 2012 at 1:08 AM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> "H.J. Lu" <hjl.tools@gmail.com> writes:
>> diff --git a/gcc/combine.c b/gcc/combine.c
>> index a07c046..b9a3589 100644
>> --- a/gcc/combine.c
>> +++ b/gcc/combine.c
>> @@ -10784,12 +10784,30 @@ gen_lowpart_for_combine (enum machine_mode omode, rtx
>> x)
>>    if (omode == imode)
>>      return x;
>>
>> -  /* Return identity if this is a CONST or symbolic reference.  */
>> -  if (omode == Pmode
>> -      && (GET_CODE (x) == CONST
>> -       || GET_CODE (x) == SYMBOL_REF
>> -       || GET_CODE (x) == LABEL_REF))
>> -    return x;
>> +  if (omode == Pmode)
>> +    {
>> +      /* Return identity if this is a symbolic reference.  */
>> +      if (GET_CODE (x) == SYMBOL_REF || GET_CODE (x) == LABEL_REF)
>> +     return x;
>> +
>> +      /* Return identity for CONST unless this is a PLUS of 2 constant
>> +      operands.  */
>> +      if (GET_CODE (x) == CONST)
>> +     {
>> +       rtx y = XEXP (x, 0);
>> +       if (GET_CODE (y) == PLUS
>> +           && ((CONST_INT_P (XEXP (y, 1))
>> +                && (GET_CODE (XEXP (y, 0)) == CONST
>> +                    || GET_CODE (XEXP (y, 0)) == SYMBOL_REF
>> +                    || GET_CODE (XEXP (y, 0)) == LABEL_REF))
>> +               || (CONST_INT_P (XEXP (y, 1))
>> +                   && (GET_CODE (XEXP (y, 0)) == CONST
>> +                       || GET_CODE (XEXP (y, 0)) == SYMBOL_REF
>> +                       || GET_CODE (XEXP (y, 0)) == LABEL_REF))))
>> +         goto fail;
>> +       return x;
>> +     }
>> +    }
>>
>>    /* We can only support MODE being wider than a word if X is a
>>       constant integer or has a mode the same size.  */
>>
>> works for the testcase.
>
> My point was that the "return x" is always wrong.  Whenever we return x
> here, we know we're returning something in a different mode from the one
> that the caller wanted.  Returning a Pmode LABEL_REF might not trigger
> that plus_constant assert, but it's still wrong.
>
> It looks like this came from the mips-rewrite branch:
>
> 2003-03-13  Richard Sandiford  <rsandifo@redhat.com>
>
>         * combine.c (gen_lowpart_for_combine): Treat the lowpart Pmode of
>         a CONST as identity.  Check the return value of gen_lowpart_common.
>
> so I can categorically confirm that the person who wrote it didn't
> know what they were doing.  It also means that this case was motivated
> by an experiment to make Pmode == DImode for n32, which we ended up
> discarding because it produced worse code.
>
> If this case really is important, we might consider using
> convert_memory_address (Pmode, x) instead.  I'm not sure whether
> that would be right for targets with address spaces though, because
> we don't know which address space is associated with the address.
> Hopefully someone who knows address spaces can comment.
>
> If it is correct, then it should probably go in gen_lowpart_common
> rather than gen_lowpart_for_combine.
>
> But given how few people have hit this, it doesn't look like a
> particularly important attempted optimisation.  I'll pre-approve
> a patch that undoes my mistake and simply removes:
>
>   /* Return identity if this is a CONST or symbolic reference.  */
>   if (omode == Pmode
>       && (GET_CODE (x) == CONST
>           || GET_CODE (x) == SYMBOL_REF
>           || GET_CODE (x) == LABEL_REF))
>     return x;
>
> Richard

This is the patch I checked in.

Thanks.

-- 
H.J.
---
gcc/

2012-08-08  Richard Sandiford  <rdsandiford@googlemail.com>
            H.J. Lu  <hongjiu.lu@intel.com>

        PR rtl-optimization/54157
        * combine.c (gen_lowpart_for_combine): Don't return identity
        for CONST or symbolic reference.

gcc/testsuite/

2012-08-08  H.J. Lu  <hongjiu.lu@intel.com>

        PR rtl-optimization/54157
        * gcc.target/i386/pr54157.c: New file.

diff --git a/gcc/combine.c b/gcc/combine.c
index 495e129..2b91eb9 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -10634,13 +10634,6 @@ gen_lowpart_for_combine (enum machine_mode
omode, rtx x)
   if (omode == imode)
     return x;

-  /* Return identity if this is a CONST or symbolic reference.  */
-  if (omode == Pmode
-      && (GET_CODE (x) == CONST
-         || GET_CODE (x) == SYMBOL_REF
-         || GET_CODE (x) == LABEL_REF))
-    return x;
-
   /* We can only support MODE being wider than a word if X is a
      constant integer or has a mode the same size.  */
   if (GET_MODE_SIZE (omode) > UNITS_PER_WORD
diff --git a/gcc/testsuite/gcc.target/i386/pr54157.c
b/gcc/testsuite/gcc.target/i386/pr54157.c
new file mode 100644
index 0000000..b5c4528
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr54157.c
@@ -0,0 +1,21 @@
+/* { dg-do compile { target { ! { ia32 } } } } */
+/* { dg-options "-O2 -mx32 -maddress-mode=long -ftree-vectorize" } */
+
+struct s2{
+  int n[24 -1][24 -1][24 -1];
+};
+
+struct test2{
+  struct s2 e;
+};
+
+struct test2 tmp2[4];
+
+void main1 ()
+{
+  int i,j;
+
+  for (i = 0; i < 24 -4; i++)
+      for (j = 0; j < 24 -4; j++)
+          tmp2[2].e.n[1][i][j] = 8;
+}

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: PATCH: PR rtl-optimization/54157: [x32] -maddress-mode=long failures
  2012-08-08 13:40             ` H.J. Lu
@ 2012-08-08 13:43               ` Uros Bizjak
  2012-08-08 13:50                 ` H.J. Lu
  0 siblings, 1 reply; 12+ messages in thread
From: Uros Bizjak @ 2012-08-08 13:43 UTC (permalink / raw)
  To: H.J. Lu; +Cc: gcc-patches, rdsandiford

On Wed, Aug 8, 2012 at 3:40 PM, H.J. Lu <hjl.tools@gmail.com> wrote:

> <rdsandiford@googlemail.com> wrote:
>> "H.J. Lu" <hjl.tools@gmail.com> writes:
>>> diff --git a/gcc/combine.c b/gcc/combine.c
>>> index a07c046..b9a3589 100644
>>> --- a/gcc/combine.c
>>> +++ b/gcc/combine.c
>>> @@ -10784,12 +10784,30 @@ gen_lowpart_for_combine (enum machine_mode omode, rtx
>>> x)
>>>    if (omode == imode)
>>>      return x;
>>>
>>> -  /* Return identity if this is a CONST or symbolic reference.  */
>>> -  if (omode == Pmode
>>> -      && (GET_CODE (x) == CONST
>>> -       || GET_CODE (x) == SYMBOL_REF
>>> -       || GET_CODE (x) == LABEL_REF))
>>> -    return x;
>>> +  if (omode == Pmode)
>>> +    {
>>> +      /* Return identity if this is a symbolic reference.  */
>>> +      if (GET_CODE (x) == SYMBOL_REF || GET_CODE (x) == LABEL_REF)
>>> +     return x;
>>> +
>>> +      /* Return identity for CONST unless this is a PLUS of 2 constant
>>> +      operands.  */
>>> +      if (GET_CODE (x) == CONST)
>>> +     {
>>> +       rtx y = XEXP (x, 0);
>>> +       if (GET_CODE (y) == PLUS
>>> +           && ((CONST_INT_P (XEXP (y, 1))
>>> +                && (GET_CODE (XEXP (y, 0)) == CONST
>>> +                    || GET_CODE (XEXP (y, 0)) == SYMBOL_REF
>>> +                    || GET_CODE (XEXP (y, 0)) == LABEL_REF))
>>> +               || (CONST_INT_P (XEXP (y, 1))
>>> +                   && (GET_CODE (XEXP (y, 0)) == CONST
>>> +                       || GET_CODE (XEXP (y, 0)) == SYMBOL_REF
>>> +                       || GET_CODE (XEXP (y, 0)) == LABEL_REF))))
>>> +         goto fail;
>>> +       return x;
>>> +     }
>>> +    }
>>>
>>>    /* We can only support MODE being wider than a word if X is a
>>>       constant integer or has a mode the same size.  */
>>>
>>> works for the testcase.
>>
>> My point was that the "return x" is always wrong.  Whenever we return x
>> here, we know we're returning something in a different mode from the one
>> that the caller wanted.  Returning a Pmode LABEL_REF might not trigger
>> that plus_constant assert, but it's still wrong.
>>
>> It looks like this came from the mips-rewrite branch:
>>
>> 2003-03-13  Richard Sandiford  <rsandifo@redhat.com>
>>
>>         * combine.c (gen_lowpart_for_combine): Treat the lowpart Pmode of
>>         a CONST as identity.  Check the return value of gen_lowpart_common.
>>
>> so I can categorically confirm that the person who wrote it didn't
>> know what they were doing.  It also means that this case was motivated
>> by an experiment to make Pmode == DImode for n32, which we ended up
>> discarding because it produced worse code.
>>
>> If this case really is important, we might consider using
>> convert_memory_address (Pmode, x) instead.  I'm not sure whether
>> that would be right for targets with address spaces though, because
>> we don't know which address space is associated with the address.
>> Hopefully someone who knows address spaces can comment.
>>
>> If it is correct, then it should probably go in gen_lowpart_common
>> rather than gen_lowpart_for_combine.
>>
>> But given how few people have hit this, it doesn't look like a
>> particularly important attempted optimisation.  I'll pre-approve
>> a patch that undoes my mistake and simply removes:
>>
>>   /* Return identity if this is a CONST or symbolic reference.  */
>>   if (omode == Pmode
>>       && (GET_CODE (x) == CONST
>>           || GET_CODE (x) == SYMBOL_REF
>>           || GET_CODE (x) == LABEL_REF))
>>     return x;
>>
>> Richard
>
> This is the patch I checked in.

Probably we need to backport this patch to 4.7, where x32 is
-maddress-mode=long by default.

Uros.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: PATCH: PR rtl-optimization/54157: [x32] -maddress-mode=long failures
  2012-08-08 13:43               ` Uros Bizjak
@ 2012-08-08 13:50                 ` H.J. Lu
  2012-08-08 15:11                   ` Richard Sandiford
  0 siblings, 1 reply; 12+ messages in thread
From: H.J. Lu @ 2012-08-08 13:50 UTC (permalink / raw)
  To: Uros Bizjak; +Cc: gcc-patches, rdsandiford

On Wed, Aug 8, 2012 at 6:43 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Wed, Aug 8, 2012 at 3:40 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>
>> <rdsandiford@googlemail.com> wrote:
>>> "H.J. Lu" <hjl.tools@gmail.com> writes:
>>>> diff --git a/gcc/combine.c b/gcc/combine.c
>>>> index a07c046..b9a3589 100644
>>>> --- a/gcc/combine.c
>>>> +++ b/gcc/combine.c
>>>> @@ -10784,12 +10784,30 @@ gen_lowpart_for_combine (enum machine_mode omode, rtx
>>>> x)
>>>>    if (omode == imode)
>>>>      return x;
>>>>
>>>> -  /* Return identity if this is a CONST or symbolic reference.  */
>>>> -  if (omode == Pmode
>>>> -      && (GET_CODE (x) == CONST
>>>> -       || GET_CODE (x) == SYMBOL_REF
>>>> -       || GET_CODE (x) == LABEL_REF))
>>>> -    return x;
>>>> +  if (omode == Pmode)
>>>> +    {
>>>> +      /* Return identity if this is a symbolic reference.  */
>>>> +      if (GET_CODE (x) == SYMBOL_REF || GET_CODE (x) == LABEL_REF)
>>>> +     return x;
>>>> +
>>>> +      /* Return identity for CONST unless this is a PLUS of 2 constant
>>>> +      operands.  */
>>>> +      if (GET_CODE (x) == CONST)
>>>> +     {
>>>> +       rtx y = XEXP (x, 0);
>>>> +       if (GET_CODE (y) == PLUS
>>>> +           && ((CONST_INT_P (XEXP (y, 1))
>>>> +                && (GET_CODE (XEXP (y, 0)) == CONST
>>>> +                    || GET_CODE (XEXP (y, 0)) == SYMBOL_REF
>>>> +                    || GET_CODE (XEXP (y, 0)) == LABEL_REF))
>>>> +               || (CONST_INT_P (XEXP (y, 1))
>>>> +                   && (GET_CODE (XEXP (y, 0)) == CONST
>>>> +                       || GET_CODE (XEXP (y, 0)) == SYMBOL_REF
>>>> +                       || GET_CODE (XEXP (y, 0)) == LABEL_REF))))
>>>> +         goto fail;
>>>> +       return x;
>>>> +     }
>>>> +    }
>>>>
>>>>    /* We can only support MODE being wider than a word if X is a
>>>>       constant integer or has a mode the same size.  */
>>>>
>>>> works for the testcase.
>>>
>>> My point was that the "return x" is always wrong.  Whenever we return x
>>> here, we know we're returning something in a different mode from the one
>>> that the caller wanted.  Returning a Pmode LABEL_REF might not trigger
>>> that plus_constant assert, but it's still wrong.
>>>
>>> It looks like this came from the mips-rewrite branch:
>>>
>>> 2003-03-13  Richard Sandiford  <rsandifo@redhat.com>
>>>
>>>         * combine.c (gen_lowpart_for_combine): Treat the lowpart Pmode of
>>>         a CONST as identity.  Check the return value of gen_lowpart_common.
>>>
>>> so I can categorically confirm that the person who wrote it didn't
>>> know what they were doing.  It also means that this case was motivated
>>> by an experiment to make Pmode == DImode for n32, which we ended up
>>> discarding because it produced worse code.
>>>
>>> If this case really is important, we might consider using
>>> convert_memory_address (Pmode, x) instead.  I'm not sure whether
>>> that would be right for targets with address spaces though, because
>>> we don't know which address space is associated with the address.
>>> Hopefully someone who knows address spaces can comment.
>>>
>>> If it is correct, then it should probably go in gen_lowpart_common
>>> rather than gen_lowpart_for_combine.
>>>
>>> But given how few people have hit this, it doesn't look like a
>>> particularly important attempted optimisation.  I'll pre-approve
>>> a patch that undoes my mistake and simply removes:
>>>
>>>   /* Return identity if this is a CONST or symbolic reference.  */
>>>   if (omode == Pmode
>>>       && (GET_CODE (x) == CONST
>>>           || GET_CODE (x) == SYMBOL_REF
>>>           || GET_CODE (x) == LABEL_REF))
>>>     return x;
>>>
>>> Richard
>>
>> This is the patch I checked in.
>
> Probably we need to backport this patch to 4.7, where x32 is
> -maddress-mode=long by default.
>

It doesn't fail on 4.7 branch since checking mode on PLUS CONST
is new on trunk.  However, I think it is a correctness issue.  Is this
OK to backport to 4.7?

Thanks.

-- 
H.J.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: PATCH: PR rtl-optimization/54157: [x32] -maddress-mode=long failures
  2012-08-08 13:50                 ` H.J. Lu
@ 2012-08-08 15:11                   ` Richard Sandiford
  2012-08-09 14:51                     ` H.J. Lu
  0 siblings, 1 reply; 12+ messages in thread
From: Richard Sandiford @ 2012-08-08 15:11 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Uros Bizjak, gcc-patches

"H.J. Lu" <hjl.tools@gmail.com> writes:
> On Wed, Aug 8, 2012 at 6:43 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
>> Probably we need to backport this patch to 4.7, where x32 is
>> -maddress-mode=long by default.
>>
>
> It doesn't fail on 4.7 branch since checking mode on PLUS CONST
> is new on trunk.  However, I think it is a correctness issue.  Is this
> OK to backport to 4.7?

Yeah, I agree we should backport it.

Richard

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: PATCH: PR rtl-optimization/54157: [x32] -maddress-mode=long failures
  2012-08-08 15:11                   ` Richard Sandiford
@ 2012-08-09 14:51                     ` H.J. Lu
  0 siblings, 0 replies; 12+ messages in thread
From: H.J. Lu @ 2012-08-09 14:51 UTC (permalink / raw)
  To: Uros Bizjak, gcc-patches, rdsandiford

On Wed, Aug 8, 2012 at 8:11 AM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> "H.J. Lu" <hjl.tools@gmail.com> writes:
>> On Wed, Aug 8, 2012 at 6:43 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
>>> Probably we need to backport this patch to 4.7, where x32 is
>>> -maddress-mode=long by default.
>>>
>>
>> It doesn't fail on 4.7 branch since checking mode on PLUS CONST
>> is new on trunk.  However, I think it is a correctness issue.  Is this
>> OK to backport to 4.7?
>
> Yeah, I agree we should backport it.
>
> Richard

I am checking this into 4.7 branch.  Tested on Linux/x32, Linux/ia32
and Linux/x86-64.

Thanks.

-- 
H.J.
---
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index bc7c36c..44b0d32 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,13 @@
+2012-08-09  H.J. Lu  <hongjiu.lu@intel.com>
+
+	Backport from mainline
+	2012-08-08  Richard Sandiford  <rdsandiford@googlemail.com>
+		    H.J. Lu  <hongjiu.lu@intel.com>
+
+	PR rtl-optimization/54157
+	* combine.c (gen_lowpart_for_combine): Don't return identity
+	for CONST or symbolic reference.
+
 2012-08-06  Uros Bizjak  <ubizjak@gmail.com>

 	Backport from mainline
diff --git a/gcc/combine.c b/gcc/combine.c
index 3d81da8a..67bd776 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -10802,13 +10802,6 @@ gen_lowpart_for_combine (enum machine_mode
omode, rtx x)
   if (omode == imode)
     return x;

-  /* Return identity if this is a CONST or symbolic reference.  */
-  if (omode == Pmode
-      && (GET_CODE (x) == CONST
-	  || GET_CODE (x) == SYMBOL_REF
-	  || GET_CODE (x) == LABEL_REF))
-    return x;
-
   /* We can only support MODE being wider than a word if X is a
      constant integer or has a mode the same size.  */
   if (GET_MODE_SIZE (omode) > UNITS_PER_WORD
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 9fd8113..ef35a62 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,11 @@
+2012-08-09  H.J. Lu  <hongjiu.lu@intel.com>
+
+	Backport from mainline
+	2012-08-08  H.J. Lu  <hongjiu.lu@intel.com>
+
+	PR rtl-optimization/54157
+	* gcc.target/i386/pr54157.c: New file.
+
 2012-08-01  Uros Bizjak  <ubizjak@gmail.com>

 	Backport from mainline
diff --git a/gcc/testsuite/gcc.target/i386/pr54157.c
b/gcc/testsuite/gcc.target/i386/pr54157.c
new file mode 100644
index 0000000..59fcd79
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr54157.c
@@ -0,0 +1,21 @@
+/* { dg-do compile { target { ! { ia32 } } } } */
+/* { dg-options "-O2 -mx32 -ftree-vectorize" } */
+
+struct s2{
+  int n[24 -1][24 -1][24 -1];
+};
+
+struct test2{
+  struct s2 e;
+};
+
+struct test2 tmp2[4];
+
+void main1 ()
+{
+  int i,j;
+
+  for (i = 0; i < 24 -4; i++)
+      for (j = 0; j < 24 -4; j++)
+          tmp2[2].e.n[1][i][j] = 8;
+}

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2012-08-09 14:51 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-01 18:41 PATCH: PR rtl-optimization/54157: [x32] -maddress-mode=long failures H.J. Lu
2012-08-01 18:59 ` Richard Sandiford
2012-08-01 19:14   ` H.J. Lu
2012-08-02 18:21     ` H.J. Lu
2012-08-05  7:47       ` Richard Sandiford
2012-08-07 19:28         ` H.J. Lu
2012-08-08  8:09           ` Richard Sandiford
2012-08-08 13:40             ` H.J. Lu
2012-08-08 13:43               ` Uros Bizjak
2012-08-08 13:50                 ` H.J. Lu
2012-08-08 15:11                   ` Richard Sandiford
2012-08-09 14:51                     ` H.J. Lu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).