diff mbox

RFA: Use gen_int_mode in plus_constant

Message ID 87wqrk5aoa.fsf@sandifor-thinkpad.stglab.manchester.uk.ibm.com
State New
Headers show

Commit Message

Richard Sandiford April 30, 2013, 2:56 p.m. UTC 
This patch fixes out the GEN_INT/gen_int_mode that Richard pointed out
in the wide-int review.  It also passes "mode" rather than "VOIDmode"
to immed_double_int_const.  (As discussed in that thread, the latter
change shouldn't make any difference in practice, but is still more
correct in principle.)

Bootstrapped & regression-tested on x86_64-linux-gnu.  OK to install?

Richard

gcc/
	* explow.c (plus_constant): Pass "mode" to immed_double_int_const.
	Use gen_int_mode rather than GEN_INT.

Comments

Richard Biener April 30, 2013, 4:01 p.m. UTC | #1 
Richard Sandiford <rsandifo@linux.vnet.ibm.com> wrote:

>This patch fixes out the GEN_INT/gen_int_mode that Richard pointed out
>in the wide-int review.  It also passes "mode" rather than "VOIDmode"
>to immed_double_int_const.  (As discussed in that thread, the latter
>change shouldn't make any difference in practice, but is still more
>correct in principle.)
>
>Bootstrapped & regression-tested on x86_64-linux-gnu.  OK to install?

Ok.

Thanks,
Richard.

>Richard
>
>gcc/
>	* explow.c (plus_constant): Pass "mode" to immed_double_int_const.
>	Use gen_int_mode rather than GEN_INT.
>
>Index: gcc/explow.c
>===================================================================
>--- gcc/explow.c	2013-02-25 09:41:58.000000000 +0000
>+++ gcc/explow.c	2013-04-30 15:52:57.270362112 +0100
>@@ -106,10 +106,10 @@ plus_constant (enum machine_mode mode, r
> 	  if (overflow)
> 	    gcc_unreachable ();
> 
>-	  return immed_double_int_const (v, VOIDmode);
>+	  return immed_double_int_const (v, mode);
> 	}
> 
>-      return GEN_INT (INTVAL (x) + c);
>+      return gen_int_mode (INTVAL (x) + c, mode);
> 
>     case CONST_DOUBLE:
>       {
>@@ -124,7 +124,7 @@ plus_constant (enum machine_mode mode, r
> 	     To fix, add constant support wider than CONST_DOUBLE.  */
> 	  gcc_assert (GET_MODE_BITSIZE (mode) <= HOST_BITS_PER_DOUBLE_INT);
> 
>-	return immed_double_int_const (v, VOIDmode);
>+	return immed_double_int_const (v, mode);
>       }
> 
>     case MEM:
Andreas Krebbel May 21, 2013, 7:37 a.m. UTC | #2 
On 30/04/13 16:56, Richard Sandiford wrote:
> This patch fixes out the GEN_INT/gen_int_mode that Richard pointed out
> in the wide-int review.  It also passes "mode" rather than "VOIDmode"
> to immed_double_int_const.  (As discussed in that thread, the latter
> change shouldn't make any difference in practice, but is still more
> correct in principle.)
> 
> Bootstrapped & regression-tested on x86_64-linux-gnu.  OK to install?
> 
> Richard
> 
> gcc/
> 	* explow.c (plus_constant): Pass "mode" to immed_double_int_const.
> 	Use gen_int_mode rather than GEN_INT.
> 
> Index: gcc/explow.c
> ===================================================================
> --- gcc/explow.c	2013-02-25 09:41:58.000000000 +0000
> +++ gcc/explow.c	2013-04-30 15:52:57.270362112 +0100
> @@ -106,10 +106,10 @@ plus_constant (enum machine_mode mode, r
>  	  if (overflow)
>  	    gcc_unreachable ();
> 
> -	  return immed_double_int_const (v, VOIDmode);
> +	  return immed_double_int_const (v, mode);
>  	}
> 
> -      return GEN_INT (INTVAL (x) + c);
> +      return gen_int_mode (INTVAL (x) + c, mode);

This calls trunc_int_for_mode which fails for mode == VOIDmode.
On s390x gcc.c-torture/compile/20021008-1.c fails due to this.

-Andreas-

> 
>      case CONST_DOUBLE:
>        {
> @@ -124,7 +124,7 @@ plus_constant (enum machine_mode mode, r
>  	     To fix, add constant support wider than CONST_DOUBLE.  */
>  	  gcc_assert (GET_MODE_BITSIZE (mode) <= HOST_BITS_PER_DOUBLE_INT);
> 
> -	return immed_double_int_const (v, VOIDmode);
> +	return immed_double_int_const (v, mode);
>        }
> 
>      case MEM:
>
Richard Sandiford May 21, 2013, 8:39 a.m. UTC | #3 
Andreas Krebbel <krebbel@linux.vnet.ibm.com> writes:
> On 30/04/13 16:56, Richard Sandiford wrote:
>> This patch fixes out the GEN_INT/gen_int_mode that Richard pointed out
>> in the wide-int review.  It also passes "mode" rather than "VOIDmode"
>> to immed_double_int_const.  (As discussed in that thread, the latter
>> change shouldn't make any difference in practice, but is still more
>> correct in principle.)
>> 
>> Bootstrapped & regression-tested on x86_64-linux-gnu.  OK to install?
>> 
>> Richard
>> 
>> gcc/
>> 	* explow.c (plus_constant): Pass "mode" to immed_double_int_const.
>> 	Use gen_int_mode rather than GEN_INT.
>> 
>> Index: gcc/explow.c
>> ===================================================================
>> --- gcc/explow.c	2013-02-25 09:41:58.000000000 +0000
>> +++ gcc/explow.c	2013-04-30 15:52:57.270362112 +0100
>> @@ -106,10 +106,10 @@ plus_constant (enum machine_mode mode, r
>>  	  if (overflow)
>>  	    gcc_unreachable ();
>> 
>> -	  return immed_double_int_const (v, VOIDmode);
>> +	  return immed_double_int_const (v, mode);
>>  	}
>> 
>> -      return GEN_INT (INTVAL (x) + c);
>> +      return gen_int_mode (INTVAL (x) + c, mode);
>
> This calls trunc_int_for_mode which fails for mode == VOIDmode.
> On s390x gcc.c-torture/compile/20021008-1.c fails due to this.

But it's invalid to pass mode == VOIDmode to plus_constant too.
Which caller is causing trouble?

Thanks,
Richard
Andreas Krebbel May 21, 2013, 9:13 a.m. UTC | #4 
On 21/05/13 10:39, Richard Sandiford wrote:
> Andreas Krebbel <krebbel@linux.vnet.ibm.com> writes:
>> On 30/04/13 16:56, Richard Sandiford wrote:
>>> This patch fixes out the GEN_INT/gen_int_mode that Richard pointed out
>>> in the wide-int review.  It also passes "mode" rather than "VOIDmode"
>>> to immed_double_int_const.  (As discussed in that thread, the latter
>>> change shouldn't make any difference in practice, but is still more
>>> correct in principle.)
>>>
>>> Bootstrapped & regression-tested on x86_64-linux-gnu.  OK to install?
>>>
>>> Richard
>>>
>>> gcc/
>>> 	* explow.c (plus_constant): Pass "mode" to immed_double_int_const.
>>> 	Use gen_int_mode rather than GEN_INT.
>>>
>>> Index: gcc/explow.c
>>> ===================================================================
>>> --- gcc/explow.c	2013-02-25 09:41:58.000000000 +0000
>>> +++ gcc/explow.c	2013-04-30 15:52:57.270362112 +0100
>>> @@ -106,10 +106,10 @@ plus_constant (enum machine_mode mode, r
>>>  	  if (overflow)
>>>  	    gcc_unreachable ();
>>>
>>> -	  return immed_double_int_const (v, VOIDmode);
>>> +	  return immed_double_int_const (v, mode);
>>>  	}
>>>
>>> -      return GEN_INT (INTVAL (x) + c);
>>> +      return gen_int_mode (INTVAL (x) + c, mode);
>>
>> This calls trunc_int_for_mode which fails for mode == VOIDmode.
>> On s390x gcc.c-torture/compile/20021008-1.c fails due to this.
> 
> But it's invalid to pass mode == VOIDmode to plus_constant too.
> Which caller is causing trouble?
> 
> Thanks,
> Richard
> 

Hi Richard,

the call comes from reload when checking a const_int memory address.

Breakpoint 1, trunc_int_for_mode (c=16, mode=VOIDmode) at /home/andreas/clean/gcc-head/gcc/explow.c:55
55        gcc_assert (SCALAR_INT_MODE_P (mode));
(gdb) bt
#0  trunc_int_for_mode (c=16, mode=VOIDmode) at /home/andreas/clean/gcc-head/gcc/explow.c:55
#1  0x00000000802b23be in gen_int_mode (c=<optimized out>, mode=<optimized out>)
    at /home/andreas/clean/gcc-head/gcc/emit-rtl.c:420
#2  0x00000000802c4e6a in plus_constant (mode=<optimized out>, x=<optimized out>, c=15)
    at /home/andreas/clean/gcc-head/gcc/explow.c:112
#3  0x00000000804cfae6 in offsettable_address_addr_space_p (strictp=strictp@entry=0,
    mode=<optimized out>, y=0x3fff7634480, as=<optimized out>)
    at /home/andreas/clean/gcc-head/gcc/recog.c:2018
#4  0x00000000804cfcb4 in offsettable_nonstrict_memref_p (op=<optimized out>)
    at /home/andreas/clean/gcc-head/gcc/recog.c:1931
#5  0x00000000804ec024 in find_reloads (insn=0x3fff7711708, replace=<optimized out>,
    ind_levels=<optimized out>, live_known=<optimized out>, reload_reg_p=<optimized out>)
    at /home/andreas/clean/gcc-head/gcc/reload.c:3395
#6  0x00000000804f86de in calculate_needs_all_insns (global=<optimized out>)
    at /home/andreas/clean/gcc-head/gcc/reload1.c:1520
#7  reload (first=0x3fff7722040, global=<optimized out>)
    at /home/andreas/clean/gcc-head/gcc/reload1.c:941
#8  0x0000000080405aa2 in do_reload () at /home/andreas/clean/gcc-head/gcc/ira.c:4653
#9  rest_of_handle_reload () at /home/andreas/clean/gcc-head/gcc/ira.c:4753
#10 0x00000000804aa42a in execute_one_pass (pass=pass@entry=0x80c580e0 <pass_reload>)
    at /home/andreas/clean/gcc-head/gcc/passes.c:2337
#11 0x00000000804aa870 in execute_pass_list (pass=0x80c580e0 <pass_reload>)
    at /home/andreas/clean/gcc-head/gcc/passes.c:2389
#12 0x00000000804aa88c in execute_pass_list (pass=0x80c587d0 <pass_rest_of_compilation>)
    at /home/andreas/clean/gcc-head/gcc/passes.c:2390
#13 0x0000000080224464 in expand_function (node=0x3fff7721000)
    at /home/andreas/clean/gcc-head/gcc/cgraphunit.c:1648
#14 0x0000000080226472 in expand_all_functions ()
    at /home/andreas/clean/gcc-head/gcc/cgraphunit.c:1752
#15 compile () at /home/andreas/clean/gcc-head/gcc/cgraphunit.c:2050
#16 0x0000000080226b98 in finalize_compilation_unit ()
    at /home/andreas/clean/gcc-head/gcc/cgraphunit.c:2127
#17 0x00000000800f2e1e in c_write_global_declarations ()
    at /home/andreas/clean/gcc-head/gcc/c/c-decl.c:10118
#18 0x000000008055a990 in compile_file () at /home/andreas/clean/gcc-head/gcc/toplev.c:558
#19 0x000000008055caf0 in do_compile () at /home/andreas/clean/gcc-head/gcc/toplev.c:1872
#20 toplev_main (argc=18, argv=0x3fffffff148) at /home/andreas/clean/gcc-head/gcc/toplev.c:1948
#21 0x000003fffdc99088 in __libc_start_main (main=0x800d4bbc <main(int, char**)>,
    argc=<optimized out>, ubp_av=0x3fffffff148, init=<optimized out>,
    fini=0x809e4a8c <__libc_csu_fini>, rtld_fini=0x3fffdfed8f4 <_dl_fini>, stack_end=0x3fffffff090)
    at libc-start.c:226
#22 0x00000000800d4c1e in _start ()
(gdb) frame 5
#5  0x00000000804ec024 in find_reloads (insn=0x3fff7711708, replace=<optimized out>,
    ind_levels=<optimized out>, live_known=<optimized out>, reload_reg_p=<optimized out>)
    at /home/andreas/clean/gcc-head/gcc/reload.c:3395
3395                                  : offsettable_nonstrict_memref_p (operand))
(gdb) p debug_rtx(insn)
(insn 7 18 12 2 (set (mem/v/c:TF (plus:DI (reg/f:DI 15 %r15)
                (const_int 160 [0xa0])) [0 val+0 S16 A64])
        (mem:TF (const_int 1 [0x1]) [0 MEM[(long double *)buf_3(D) + 1B]+0 S16 A64]))
/home/andreas/clean/gcc-head/gcc/testsuite/gcc.c-torture/compile/20021008-1.c:9 78 {*movtf_64}
     (nil))

Bye,

-Andreas-
diff mbox

Patch

Index: gcc/explow.c
===================================================================
--- gcc/explow.c	2013-02-25 09:41:58.000000000 +0000
+++ gcc/explow.c	2013-04-30 15:52:57.270362112 +0100
@@ -106,10 +106,10 @@  plus_constant (enum machine_mode mode, r
 	  if (overflow)
 	    gcc_unreachable ();
 
-	  return immed_double_int_const (v, VOIDmode);
+	  return immed_double_int_const (v, mode);
 	}
 
-      return GEN_INT (INTVAL (x) + c);
+      return gen_int_mode (INTVAL (x) + c, mode);
 
     case CONST_DOUBLE:
       {
@@ -124,7 +124,7 @@  plus_constant (enum machine_mode mode, r
 	     To fix, add constant support wider than CONST_DOUBLE.  */
 	  gcc_assert (GET_MODE_BITSIZE (mode) <= HOST_BITS_PER_DOUBLE_INT);
 
-	return immed_double_int_const (v, VOIDmode);
+	return immed_double_int_const (v, mode);
       }
 
     case MEM: