diff mbox

ptx preliminary rtl patches [1/4]

Message ID 5411A2A6.1030303@codesourcery.com
State New
Headers show

Commit Message

Bernd Schmidt Sept. 11, 2014, 1:24 p.m. UTC 
The nvptx backend is somewhat unusual in that call insns set a pseudo. 
The combiner is surprised by this and allows combining them into other 
insns, which remain as INSN rather than CALL_INSN. Aborts ensue.

Bootstrapped and tested on x86_64-linux, together with the other 
patches.  Ok?


Bernd

Comments

Bernd Schmidt Oct. 29, 2014, 11:48 p.m. UTC | #1 
On 09/11/2014 03:24 PM, Bernd Schmidt wrote:
> The nvptx backend is somewhat unusual in that call insns set a pseudo.
> The combiner is surprised by this and allows combining them into other
> insns, which remain as INSN rather than CALL_INSN. Aborts ensue.
>
> Bootstrapped and tested on x86_64-linux, together with the other
> patches.  Ok?

Ping.


Bernd
Jeff Law Oct. 30, 2014, 3:08 a.m. UTC | #2 
On 09/11/14 07:24, Bernd Schmidt wrote:
> The nvptx backend is somewhat unusual in that call insns set a pseudo.
> The combiner is surprised by this and allows combining them into other
> insns, which remain as INSN rather than CALL_INSN. Aborts ensue.
>
> Bootstrapped and tested on x86_64-linux, together with the other
> patches.  Ok?
OK.
jeff
Segher Boessenkool Oct. 30, 2014, 5:36 p.m. UTC | #3 
On Thu, Sep 11, 2014 at 03:24:54PM +0200, Bernd Schmidt wrote:
> The nvptx backend is somewhat unusual in that call insns set a pseudo. 
> The combiner is surprised by this and allows combining them into other 
> insns, which remain as INSN rather than CALL_INSN. Aborts ensue.

distribute_notes has code for when I2 is a CALL.  This suggests that
on some other targets there can be useful combinations made, so it
isn't in that case terribly nice to disable all combinations with CALLs
as I0 or I1 or I2.

Or it's dead code :-)


Segher
Jeff Law Oct. 30, 2014, 9:57 p.m. UTC | #4 
On 10/30/14 11:36, Segher Boessenkool wrote:
> On Thu, Sep 11, 2014 at 03:24:54PM +0200, Bernd Schmidt wrote:
>> The nvptx backend is somewhat unusual in that call insns set a pseudo.
>> The combiner is surprised by this and allows combining them into other
>> insns, which remain as INSN rather than CALL_INSN. Aborts ensue.
>
> distribute_notes has code for when I2 is a CALL.  This suggests that
> on some other targets there can be useful combinations made, so it
> isn't in that case terribly nice to disable all combinations with CALLs
> as I0 or I1 or I2.
>
> Or it's dead code :-)
Most likely it's dead code.

jeff
Bernd Schmidt Oct. 31, 2014, 5:22 p.m. UTC | #5 
On 10/30/2014 06:36 PM, Segher Boessenkool wrote:
> On Thu, Sep 11, 2014 at 03:24:54PM +0200, Bernd Schmidt wrote:
>> The nvptx backend is somewhat unusual in that call insns set a pseudo.
>> The combiner is surprised by this and allows combining them into other
>> insns, which remain as INSN rather than CALL_INSN. Aborts ensue.
>
> distribute_notes has code for when I2 is a CALL.  This suggests that
> on some other targets there can be useful combinations made, so it
> isn't in that case terribly nice to disable all combinations with CALLs
> as I0 or I1 or I2.
>
> Or it's dead code :-)

I suspect the latter. As far as I can tell the patch does not change 
code generation on x86_64 at least (on a large set of input files 
including gcc, spec2k and linux-kernel).


Bernd
diff mbox

Patch

    	* combine.c (try_combine): Don't allow a call as one of the source
    	insns.

diff --git a/gcc/combine.c b/gcc/combine.c
index 0ec7f85..fe95b41 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -2543,7 +2543,10 @@  try_combine (rtx_insn *i3, rtx_insn *i2, rtx_insn *i1, rtx_insn *i0,
 
   /* Exit early if one of the insns involved can't be used for
      combinations.  */
-  if (cant_combine_insn_p (i3)
+  if (CALL_P (i2)
+      || (i1 && CALL_P (i1))
+      || (i0 && CALL_P (i0))
+      || cant_combine_insn_p (i3)
       || cant_combine_insn_p (i2)
       || (i1 && cant_combine_insn_p (i1))
       || (i0 && cant_combine_insn_p (i0))