[Bug target/27282] [4.2 regression] ICE in final_scan_insn, at final.c:2448 - could not split insn

[bonzini at gnu dot org]

--- Comment #11 from bonzini at gnu dot org  2006-04-26 07:13 ---
unassigning since David and Andrew's patch works, but mine does not.

My patch BTW could be a minor cleanup, but it is not even necessary because
GEN_INT (trunc_int_for_mode (...)) does not drop sign bits (it just introduces
unnecessary but harmless casts between unsigned and signed HOST_WIDE_INTs).


-- 

bonzini at gnu dot org changed:

   What|Removed |Added

 AssignedTo|bonzini at gnu dot org  |unassigned at gcc dot gnu
   ||dot org
 Status|ASSIGNED|NEW


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27282

[Bug target/27282] [4.2 regression] ICE in final_scan_insn, at final.c:2448 - could not split insn

[dje at gcc dot gnu dot org]

--- Comment #12 from dje at gcc dot gnu dot org  2006-04-26 14:33 ---
Subject: Bug 27282

Author: dje
Date: Wed Apr 26 14:33:49 2006
New Revision: 113275

URL: http://gcc.gnu.org/viewcvs?root=gccview=revrev=113275
Log:
2006-04-26  David Edelsohn  [EMAIL PROTECTED]
Paolo Bonzini  [EMAIL PROTECTED]

PR middle-end/27282
* combine.c (simplify_and_const_int_1): Use gen_int_mode.
(simplify_and_const_int): Same.

Modified:
trunk/gcc/ChangeLog
trunk/gcc/combine.c


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27282

[Bug target/27282] [4.2 regression] ICE in final_scan_insn, at final.c:2448 - could not split insn

[pinskia at gcc dot gnu dot org]

--- Comment #13 from pinskia at gcc dot gnu dot org  2006-04-26 17:11 
---
Fixed.


-- 

pinskia at gcc dot gnu dot org changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution||FIXED


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27282

[Bug target/27282] [4.2 regression] ICE in final_scan_insn, at final.c:2448 - could not split insn

[dje at gcc dot gnu dot org]

--- Comment #14 from dje at gcc dot gnu dot org  2006-04-26 17:57 ---
Subject: Bug 27282

Author: dje
Date: Wed Apr 26 17:57:03 2006
New Revision: 113278

URL: http://gcc.gnu.org/viewcvs?root=gccview=revrev=113278
Log:
PR middle-end/27282
* gcc.c-torture/compile/pr27282.c: New test

Added:
trunk/gcc/testsuite/gcc.c-torture/compile/pr27282.c
Modified:
trunk/gcc/testsuite/ChangeLog


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27282

[Bug target/27282] [4.2 regression] ICE in final_scan_insn, at final.c:2448 - could not split insn

[bonzini at gnu dot org]

--- Comment #5 from bonzini at gnu dot org  2006-04-25 06:37 ---
Sure.  The code meant to do so using trunc_int_for_mode, but it does not work
because constop is unsigned.  The trunc_int_for_mode was introduced in
http://gcc.gnu.org/ml/gcc-patches/2002-01/msg01397.html to cure a similar ICE;
and the bug is latent on all branches since it dates back (through various
changes, notably r49112 by amodra) to the dawn of the original old-gcc
repository.

Bootstrapping this patch on i686-pc-linux-gnu, ok if it passes?  What about
4.1?

Index: gcc-fwprop/gcc/combine.c
===
--- gcc-fwprop/gcc/combine.c(revision 113024)
+++ gcc-fwprop/gcc/combine.c(working copy)
@@ -8190,6 +8190,5 @@ simplify_and_const_int_1 (enum machine_m
 return NULL_RTX;

   /* Otherwise, return an AND.  */
-  constop = trunc_int_for_mode (constop, mode);
-  return simplify_gen_binary (AND, mode, varop, GEN_INT (constop));
+  return simplify_gen_binary (AND, mode, varop, gen_int_mode (constop, mode));
 }

Paolo


-- 

bonzini at gnu dot org changed:

   What|Removed |Added

 AssignedTo|unassigned at gcc dot gnu   |bonzini at gnu dot org
   |dot org |
 Status|NEW |ASSIGNED
   Last reconfirmed|2006-04-25 00:53:57 |2006-04-25 06:37:18
   date||


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27282

[Bug target/27282] [4.2 regression] ICE in final_scan_insn, at final.c:2448 - could not split insn

[roger at eyesopen dot com]

--- Comment #6 from roger at eyesopen dot com  2006-04-25 14:09 ---
Paolo's fix looks good to me.  The bugzilla PR shows that this is a 4.2
regression, probably due to the more aggressive RTL optimizations on mainline.
So I'll preapprove Paolo's fix for mainline (please post the version you
commit and a new testcase when you commit it).

As for 4.1, do we have an example of a failure or wrong code generation
against the branch?  I can't tell from bugzilla whether this is safely
latent in 4.0 and 4.1, or just hasn't been investigated there yet
(known to work is blank, but the summary only lists [4.2]).


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27282

[Bug target/27282] [4.2 regression] ICE in final_scan_insn, at final.c:2448 - could not split insn

[dje at watson dot ibm dot com]

--- Comment #7 from dje at watson dot ibm dot com  2006-04-25 15:21 ---
Subject: Re:  [4.2 regression] ICE in final_scan_insn, at final.c:2448 - could
not split insn 

The patch may be necessary, but does not fix the testcase.  The testcase
needs the patch that Andrew originally tested:

Index: combine.c
===
--- combine.c   (revision 113239)
+++ combine.c   (working copy)
@@ -8210,7 +8209,8 @@ simplify_and_const_int (rtx x, enum mach
 return tem;

   if (!x)
-x = simplify_gen_binary (AND, GET_MODE (varop), varop, GEN_INT (constop));
+x = simplify_gen_binary (AND, GET_MODE (varop), varop,
+gen_int_mode (constop, mode));
   if (GET_MODE (x) != mode)
 x = gen_lowpart (mode, x);
   return x;


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27282

[Bug target/27282] [4.2 regression] ICE in final_scan_insn, at final.c:2448 - could not split insn

[roger at eyesopen dot com]

--- Comment #8 from roger at eyesopen dot com  2006-04-25 15:41 ---
Grr.  David's patch is also good.  Perhaps better if we follow the usual
protocol of posting patches to gcc-patches *after* bootstrap and regression
testing, for review and approval.  Posting untested patch fragments to
bugzilla without ChangeLog entries and asking for preapproval etc... seems
to, in this instance at least, demonstrate why GCC has the contribution
protocols that it has.

Thanks to David for catching this.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27282

[Bug target/27282] [4.2 regression] ICE in final_scan_insn, at final.c:2448 - could not split insn

[dje at gcc dot gnu dot org]

--- Comment #9 from dje at gcc dot gnu dot org  2006-04-25 16:09 ---
By the way, while Andrew and my patch does produce correct code, it reverts to
the original behavior of the constant propagating into the AND between life2
and lreg, not between life1 and combine.  Somehow combine is able to form the
AND with the wrong constant that does not match the pattern but cannot form the
AND with the correct, sign-extended constant.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27282

[Bug target/27282] [4.2 regression] ICE in final_scan_insn, at final.c:2448 - could not split insn

[janis at gcc dot gnu dot org]

--- Comment #10 from janis at gcc dot gnu dot org  2006-04-25 21:16 ---
A regression hunt of trunk on powerpc-linux using mini.c with -O2 identified:

http://gcc.gnu.org/viewcvs?view=revrev=109016

r109016 | bonzini | 2005-12-23 16:07:53 + (Fri, 23 Dec 2005)


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27282

[Bug target/27282] [4.2 regression] ICE in final_scan_insn, at final.c:2448 - could not split insn

[pinskia at gcc dot gnu dot org]

-- 

pinskia at gcc dot gnu dot org changed:

   What|Removed |Added

 CC||pinskia at gcc dot gnu dot
   ||org
   Keywords||ice-on-valid-code
Summary|[4.2 regression, powerpc]   |[4.2 regression] ICE in
   |ICE in final_scan_insn, at  |final_scan_insn, at
   |final.c:2448 - could not|final.c:2448 - could not
   |split insn  |split insn
   Target Milestone|--- |4.2.0


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27282

[Bug target/27282] [4.2 regression] ICE in final_scan_insn, at final.c:2448 - could not split insn

[pinskia at gcc dot gnu dot org]

--- Comment #2 from pinskia at gcc dot gnu dot org  2006-04-25 00:46 ---
(insn:HI 29 28 62 3 (parallel [
(set (reg:SI 134)
(and:SI (reg:SI 130)
(const_int 3791650816 [0xe200])))
(clobber (scratch:CC))
]) 102 {andsi3} (nil)
(expr_list:REG_DEAD (reg:SI 130)
(expr_list:REG_UNUSED (scratch:CC)
(nil


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27282

[Bug target/27282] [4.2 regression] ICE in final_scan_insn, at final.c:2448 - could not split insn

[pinskia at gcc dot gnu dot org]

--- Comment #3 from pinskia at gcc dot gnu dot org  2006-04-25 00:53 ---
Confirmed, reload is the one which is creating the (set reg const_int).


-- 

pinskia at gcc dot gnu dot org changed:

   What|Removed |Added

 Status|UNCONFIRMED |NEW
 Ever Confirmed|0   |1
   Last reconfirmed|-00-00 00:00:00 |2006-04-25 00:53:57
   date||


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27282

[Bug target/27282] [4.2 regression] ICE in final_scan_insn, at final.c:2448 - could not split insn

[dje at gcc dot gnu dot org]

--- Comment #4 from dje at gcc dot gnu dot org  2006-04-25 04:24 ---
This optimization occurred in 4.1 between life2 and lreg:

life2:
(insn 24 23 25 2 (set (reg:SI 126)
(const_int -503316480 [0xe200])) 279 {*movsi_internal1}
(nil)
(nil))

(insn 30 29 31 2 (parallel [
(set (reg:SI 134)
(and:SI (reg:SI 130)
(reg:SI 126)))
(clobber (scratch:CC))
]) 98 {andsi3} (insn_list:REG_DEP_TRUE 24 (insn_list:REG_DEP_TRUE 29
(nil)))
(expr_list:REG_UNUSED (scratch:CC)
(expr_list:REG_DEAD (reg:SI 130)
(expr_list:REG_DEAD (reg:SI 126)
(expr_list:REG_UNUSED (scratch:CC)
(nil))

lreg:
(insn 30 29 31 2 (parallel [
(set (reg:SI 134)
(and:SI (reg:SI 130)
(const_int -503316480 [0xe200])))
(clobber (scratch:CC))
]) 98 {andsi3} (insn_list:REG_DEP_TRUE 24 (insn_list:REG_DEP_TRUE 29
(nil)))
(expr_list:REG_UNUSED (scratch:CC)
(expr_list:REG_DEAD (reg:SI 130)
(expr_list:REG_UNUSED (scratch:CC)
(nil)

and the constant remained properly sign extended through combine.

In 4.2, the value is correct in life1:
(insn 23 22 24 4 (set (reg:SI 126)
(const_int -503316480 [0xe200])) 310 {*movsi_internal1}
(nil)
(nil))

(insn 28 27 29 4 (parallel [
(set (reg:SI 130)
(and:SI (reg:SI 131)
(reg:SI 126)))
(clobber (scratch:CC))
]) 129 {andsi3} (insn_list:REG_DEP_TRUE 27 (nil))
(expr_list:REG_DEAD (reg:SI 131)
(expr_list:REG_DEAD (reg:SI 126)
(expr_list:REG_UNUSED (scratch:CC)
(nil)

but wrong in combine:
(insn 29 28 30 4 (parallel [
(set (reg:SI 134)
(and:SI (reg:SI 130)
(const_int 3791650816 [0xe200])))
(clobber (scratch:CC))
]) 129 {andsi3} (insn_list:REG_DEP_TRUE 28 (nil))
(expr_list:REG_UNUSED (scratch:CC)
(expr_list:REG_DEAD (reg:SI 130)
(nil

This optimization is occurring in an earlier pass, but the constant is being
regenerated incorrectly.

I'm not sure if this is related to simplify_and_const_int or
simplify_const_binary_operation.  I do notice that simplify_and_const_int is
invoked in simplify_logical as

 x = simplify_and_const_int (x, mode, op0, INTVAL (op1));

while simplify_and_const_int signature is

simplify_and_const_int (rtx x, enum machine_mode mode, rtx varop,
unsigned HOST_WIDE_INT constop)

with an unsigned HWI argument.  It then reconstructs the argument as

x = simplify_gen_binary (AND, GET_MODE (varop), varop, GEN_INT (constop));

Apparently changing the GEN_INT() to gen_int_mode() fixes the testcase.  The
current logic is dropping sign bits on the floor.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27282