[Bug rtl-optimization/56195] [4.8 Regression] Error: incorrect register `%rdi' used with `l' suffix (at -O2)

2013-02-08 Thread jakub at gcc dot gnu.org 


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



--- Comment #8 from Jakub Jelinek jakub at gcc dot gnu.org 2013-02-08 
15:19:05 UTC ---

Author: jakub

Date: Fri Feb  8 15:19:02 2013

New Revision: 195891



URL: http://gcc.gnu.org/viewcvs?root=gccview=revrev=195891

Log:

PR rtl-optimization/56195

* lra-constraints.c (get_reload_reg): Don't reuse regs

if they have smaller mode than requested, if they have

wider mode than requested, try to return a SUBREG.



* gcc.dg/torture/pr56195.c: New test.



Added:

trunk/gcc/testsuite/gcc.dg/torture/pr56195.c

Modified:

trunk/gcc/ChangeLog

trunk/gcc/lra-constraints.c

trunk/gcc/testsuite/ChangeLog

[Bug rtl-optimization/56195] [4.8 Regression] Error: incorrect register `%rdi' used with `l' suffix (at -O2)

2013-02-08 Thread jakub at gcc dot gnu.org 


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



Jakub Jelinek jakub at gcc dot gnu.org changed:



   What|Removed |Added



 Status|NEW |RESOLVED

 Resolution||FIXED

 AssignedTo|unassigned at gcc dot   |jakub at gcc dot gnu.org

   |gnu.org |



--- Comment #9 from Jakub Jelinek jakub at gcc dot gnu.org 2013-02-08 
15:24:40 UTC ---

Fixed.

[Bug rtl-optimization/56195] [4.8 Regression] Error: incorrect register `%rdi' used with `l' suffix (at -O2)

2013-02-07 Thread vmakarov at redhat dot com 


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



Vladimir Makarov vmakarov at redhat dot com changed:



   What|Removed |Added



 CC||vmakarov at redhat dot com



--- Comment #4 from Vladimir Makarov vmakarov at redhat dot com 2013-02-07 
19:24:58 UTC ---

(In reply to comment #3)

 I'd say the bug is in get_reload_reg.

 Changing pseudo 118 in operand 0 of insn 90 on equiv 0

 Changing address in insn 90 r59:DI -- no change

 Changing pseudo 59 in address of insn 90 on equiv 0

   Creating newreg=137, assigning class GENERAL_REGS to address r137

  Choosing alt 1 in insn 90:  (0) r  (1) rm

  Reuse r137 for reload 0, change to class INDEX_REGS for r137

90: flags:CCGC=cmp(r137:DI,[r137:DI])

 Inserting insn reload before:

   256: r137:DI=0

 

 

 3065  if (get_reload_reg (type, mode, old, goal_alt[i], , new_reg)

 3066   type != OP_OUT)

 

 calls it with

 type=OP_IN, mode=SImode, original=const0_rtx, rclass=GENERAL_REGS

 but returns new_reg = (reg:DI 137).

 That is because:

   if (rtx_equal_p (curr_insn_input_reloads[i].input, original)

in_class_p (curr_insn_input_reloads[i].reg, rclass, new_class))

 doesn't check any mode if original (and curr_insn_input_reloads[i].input) are

 VOIDmode as in this case.  So, either this can be fixed by doing:

if (rtx_equal_p (curr_insn_input_reloads[i].input, original)

 -   in_class_p (curr_insn_input_reloads[i].reg, rclass, new_class))

 +   in_class_p (curr_insn_input_reloads[i].reg, rclass, new_class)

 +   GET_MODE (curr_insn_input_reloads[i].reg) == mode)

 , or we could try better, if the GET_MODE (curr_insn_input_reloads[i].reg)

 is wider than mode, see if we can create a lowpart subreg thereof and return

 that, and only give up (i.e. continue looping) if creation of the lowpart

 subreg for some reason failed.

 

 Vlad, what do you think?



I think, the second solution with lowpart is better.



Would you like to make a patch or may be you prefer that I work on it?

[Bug rtl-optimization/56195] [4.8 Regression] Error: incorrect register `%rdi' used with `l' suffix (at -O2)

2013-02-07 Thread jakub at gcc dot gnu.org 


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



--- Comment #5 from Jakub Jelinek jakub at gcc dot gnu.org 2013-02-07 
19:47:04 UTC ---

So something like:



--- lra-constraints.c.jj2013-02-07 18:34:39.0 +0100

+++ lra-constraints.c2013-02-07 20:41:17.051986353 +0100

@@ -421,7 +421,21 @@ get_reload_reg (enum op_type type, enum

   if (rtx_equal_p (curr_insn_input_reloads[i].input, original)

in_class_p (curr_insn_input_reloads[i].reg, rclass, new_class))

 {

-  *result_reg = curr_insn_input_reloads[i].reg;

+  rtx reg = curr_insn_input_reloads[i].reg;

+  /* If input is equal to original and both are VOIDmode,

+ GET_MODE (reg) might be still different from mode.

+ Ensure we don't return *result_reg with wrong mode.  */

+  if (GET_MODE (reg) != mode)

+{

+  if (GET_MODE_SIZE (GET_MODE (reg))  GET_MODE_SIZE (mode))

+continue;

+  reg = simplify_subreg (mode, reg, GET_MODE (reg),

+ subreg_lowpart_offset (mode,

+GET_MODE (reg)));

+  if (reg == NULL_RTX || !REG_P (reg))

+continue;

+}

+  *result_reg = reg;

   regno = REGNO (*result_reg);

   if (lra_dump_file != NULL)

 {



?

[Bug rtl-optimization/56195] [4.8 Regression] Error: incorrect register `%rdi' used with `l' suffix (at -O2)

2013-02-07 Thread jakub at gcc dot gnu.org 


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



--- Comment #6 from Jakub Jelinek jakub at gcc dot gnu.org 2013-02-07 
20:02:20 UTC ---

Actually, that one doesn't really work, because we have pseudo rather than hard

reg at that point, which will never simplify.



With this:



--- lra-constraints.c.jj2013-02-07 18:34:39.0 +0100

+++ lra-constraints.c2013-02-07 20:58:25.558920536 +0100

@@ -421,8 +421,20 @@ get_reload_reg (enum op_type type, enum

   if (rtx_equal_p (curr_insn_input_reloads[i].input, original)

in_class_p (curr_insn_input_reloads[i].reg, rclass, new_class))

 {

-  *result_reg = curr_insn_input_reloads[i].reg;

-  regno = REGNO (*result_reg);

+  rtx reg = curr_insn_input_reloads[i].reg;

+  regno = REGNO (reg);

+  /* If input is equal to original and both are VOIDmode,

+ GET_MODE (reg) might be still different from mode.

+ Ensure we don't return *result_reg with wrong mode.  */

+  if (GET_MODE (reg) != mode)

+{

+  if (GET_MODE_SIZE (GET_MODE (reg))  GET_MODE_SIZE (mode))

+continue;

+  reg = lowpart_subreg (mode, reg, GET_MODE (reg));

+  if (reg == NULL_RTX || GET_CODE (reg) != SUBREG)

+continue;

+}

+  *result_reg = reg;

   if (lra_dump_file != NULL)

 {

   fprintf (lra_dump_file,  Reuse r%d for reload , regno);



the assembly difference is:



-cmpl(%rdi), %rdi

+cmpl(%rdi), %edi



which is desirable in this case, but not sure if all get_reload_reg callers

will grok a SUBREG instead of REG returned in *result_reg.

[Bug rtl-optimization/56195] [4.8 Regression] Error: incorrect register `%rdi' used with `l' suffix (at -O2)

2013-02-07 Thread vmakarov at redhat dot com 


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



--- Comment #7 from Vladimir Makarov vmakarov at redhat dot com 2013-02-07 
20:08:47 UTC ---

(In reply to comment #6)

 Actually, that one doesn't really work, because we have pseudo rather than 
 hard

 reg at that point, which will never simplify.

 

 With this:

 

 --- lra-constraints.c.jj2013-02-07 18:34:39.0 +0100

 +++ lra-constraints.c2013-02-07 20:58:25.558920536 +0100

 @@ -421,8 +421,20 @@ get_reload_reg (enum op_type type, enum

if (rtx_equal_p (curr_insn_input_reloads[i].input, original)

 in_class_p (curr_insn_input_reloads[i].reg, rclass, new_class))

  {

 -  *result_reg = curr_insn_input_reloads[i].reg;

 -  regno = REGNO (*result_reg);

 +  rtx reg = curr_insn_input_reloads[i].reg;

 +  regno = REGNO (reg);

 +  /* If input is equal to original and both are VOIDmode,

 + GET_MODE (reg) might be still different from mode.

 + Ensure we don't return *result_reg with wrong mode.  */

 +  if (GET_MODE (reg) != mode)

 +{

 +  if (GET_MODE_SIZE (GET_MODE (reg))  GET_MODE_SIZE (mode))

 +continue;

 +  reg = lowpart_subreg (mode, reg, GET_MODE (reg));

 +  if (reg == NULL_RTX || GET_CODE (reg) != SUBREG)

 +continue;

 +}

 +  *result_reg = reg;

if (lra_dump_file != NULL)

  {

fprintf (lra_dump_file,  Reuse r%d for reload , regno);

 

 the assembly difference is:

 

 -cmpl(%rdi), %rdi

 +cmpl(%rdi), %edi

 

 which is desirable in this case, but not sure if all get_reload_reg callers

 will grok a SUBREG instead of REG returned in *result_reg.



This version of patch looks ok for me.  I have no worry about get_reload_reg

callers.  It should work fine (that is a difference from reload pass when you

should care about secondary reloads etc).



Thanks for working on this, Jakub,