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

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

[antoine.balestrat at gmail dot com]

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

             Bug #: 56195
           Summary: Error: incorrect register `%rdi' used with `l' suffix
                    (at -O2)
    Classification: Unclassified
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: antoine.balestrat@gmail.com


Using GCC 4.8.0 as of 20130203 :

$ xgcc -v
Using built-in specs.
COLLECT_GCC=/home/merkil/gcc/dist/bin/gcc
COLLECT_LTO_WRAPPER=/home/merkil/gcc/dist/libexec/gcc/x86_64-unknown-linux-gnu/4.8.0/lto-wrapper
Target: x86_64-unknown-linux-gnu
Configured with: ../srcdir/configure CFLAGS='-O2 -pipe' CXXFLAGS='-O2 -pipe'
--enable-languages=c --prefix=/home/merkil/gcc/dist
Thread model: posix
gcc version 4.8.0 20130203 (experimental) (GCC) 

$ cat reg.c
int i, a, t, b, c, d, e, f, j, *h;

void fn(void)
{
    if(b)
    {
        int *p, *q;
        char g;

        if(f++)
            for(;; e++);
lbl:
        for(b = 0; b < 2; b++)
            t /= d + 1 ? : i || a < c < (d = f) ? : 1 | (j = 2);

        *p = g >= *q ^ c != a ^ *p;

        if(!e)
        {
            q = p;
            goto lbl;
        }
    }

    if(h++)
        goto lbl;
}

$ xgcc -O2 reg.c
{standard input}: Assembler messages:
{standard input}:112: Error: incorrect register `%rdi' used with `l' suffix

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

[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
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org,
                   |                            |vmakarov at gcc dot gnu.org
   Target Milestone|---                         |4.8.0
            Summary|Error: incorrect register   |[4.8 Regression] Error:
                   |`%rdi' used with `l' suffix |incorrect register `%rdi'
                   |(at -O2)                    |used with `l' suffix (at
                   |                            |-O2)

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-02-03 21:01:59 UTC ---
Started with LRA merge (
http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=192719 ).
(define_insn "*cmp<mode>_1"
  [(set (reg FLAGS_REG)
        (compare (match_operand:SWI 0 "nonimmediate_operand" "<r>m,<r>")
                 (match_operand:SWI 1 "<general_operand>" "<r><i>,<r>m")))]
  "ix86_match_ccmode (insn, CCmode)"
  "cmp{<imodesuffix>}\t{%1, %0|%0, %1}"
  [(set_attr "type" "icmp")
   (set_attr "mode" "<MODE>")])
In *.ira dump the insn looks still correct:
(insn 90 87 91 22 (set (reg:CCGC 17 flags)
        (compare:CCGC (reg:SI 118 [ D.1797 ])
            (mem:SI (reg/v/f:DI 59 [ q ]) [2 *q_1+0 S4 A32]))) pr56195.c:17 7
{*cmpsi_1}
     (nil))
but in *.reload dump:
(insn 90 256 91 22 (set (reg:CCGC 17 flags)
        (compare:CCGC (reg:DI 5 di [137])
            (mem:SI (reg:DI 5 di [137]) [2 *q_1+0 S4 A32]))) pr56195.c:17 7
{*cmpsi_1}
     (expr_list:REG_DEAD (reg:DI 5 di [137])
        (nil)))

so I wonder how could it match that way and survive till end of compilation.

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

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|                            |x86_64-*-*
           Priority|P3                          |P1
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2013-02-04
     Ever Confirmed|0                           |1

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

[jakub at gcc dot gnu.org]

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

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-02-05 13:56:56 UTC ---
Seems this is heavily related to the uninitialized uses, in
(insn 90 87 91 22 (set (reg:CCGC 17 flags)
        (compare:CCGC (reg:SI 118 [ D.1797 ])
            (mem:SI (reg/v/f:DI 59 [ q ]) [2 *q_1+0 S4 A32]))) pr56195.c:17 7
{*cmpsi_1}
     (nil))
both SI 118 and DI 59 pseudos are preinitialized to const0_rtx because both the
address to read from and the other value are uninitialized.  Guess LRA figures
out that both operands are equal to const0_rtx, just doesn't handle the
different modes properly (it can be cmpl (%rdi), %edi ).

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

[jakub at gcc dot gnu.org]

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-02-05 16:40:17 UTC ---
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?

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

[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)

[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.jj    2013-02-07 18:34:39.000000000 +0100
+++ lra-constraints.c    2013-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)

[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.jj    2013-02-07 18:34:39.000000000 +0100
+++ lra-constraints.c    2013-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)

[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.jj    2013-02-07 18:34:39.000000000 +0100
> +++ lra-constraints.c    2013-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,

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

[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=gcc&view=rev&rev=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)

[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.