[Bug middle-end/33713] New: [4.3 Regression] can't find a register in class 'GENERAL_REGS' while reloading 'asm'

[Bug middle-end/33713] New: [4.3 Regression] can't find a register in class 'GENERAL_REGS' while reloading 'asm'

[rguenth at gcc dot gnu dot org]

libtheora has

typedef int int16_t __attribute__ ((__mode__ (__HI__)));
typedef int16_t ogg_int16_t;
void oc_frag_recon_inter2_mmx(unsigned char *_dst,int _dst_ystride,
 const unsigned char *_src1,int _src1_ystride,const unsigned char *_src2,
 int _src2_ystride,const ogg_int16_t *_residue)
{
  long a;
  int i;
  __asm__ __volatile__(
    "pxor %%mm0,%%mm0\n\t"
    "movq (%[src1]),%%mm2\n\t"
    :[src1]"+r"(_src1)
    :
  );
  for(i=8;i-->0;){
    __asm__ __volatile__(
     "movq (%[src2]),%%mm4\n\t"
     "movq %%mm2,%%mm3\n\t"
     "movq %%mm4,%%mm5\n\t"
     "mov %[src1_ystride],%[a]\n\t"
     "punpcklbw %%mm0,%%mm2\n\t"
     "punpcklbw %%mm0,%%mm4\n\t"
     "lea (%[src1],%[a]),%[src1]\n\t"
     "punpckhbw %%mm0,%%mm3\n\t"
     "punpckhbw %%mm0,%%mm5\n\t"
     "mov %[src2_ystride],%[a]\n\t"
     "paddsw %%mm2,%%mm4\n\t"
     "paddsw %%mm3,%%mm5\n\t"
     "lea (%[src2],%[a]),%[src2]\n\t"
     "movq (%[src1]),%%mm2\n\t"
     "psrlw $1,%%mm4\n\t"
     "psrlw $1,%%mm5\n\t"
     "paddsw (%[residue]),%%mm4\n\t"
     "paddsw 8(%[residue]),%%mm5\n\t"
     "mov %[dst_ystride],%[a]\n\t"
     "packuswb %%mm5,%%mm4\n\t"
     "lea 0x10(%[residue]),%[residue]\n\t"
     "movq %%mm4,(%[dst])\n\t"
     "lea (%[dst],%[a]),%[dst]\n\t"
     :[a]"=&a"(a),[dst]"+r"(_dst),[residue]"+r"(_residue),
      [src1]"+r"(_src1),[src2]"+r"(_src2)
     :[dst_ystride]"m"((long)_dst_ystride),
      [src1_ystride]"m"((long)_src1_ystride),
      [src2_ystride]"m"((long)_src2_ystride)
     :"memory"
   );
  }
}

which gets both asms rejected with trunk (r129166) but are accepted with 4.2:

./cc1 -fpreprocessed mmxfrag.i -quiet -march=i586 -mtune=i686 -O -version
-fforce-addr -fomit-frame-pointer -fPIC -o /dev/null -m32
mmxfrag.i: In function 'oc_frag_recon_inter2_mmx':
mmxfrag.i:16: error: can't find a register in class 'GENERAL_REGS' while
reloading 'asm'
mmxfrag.i:9: error: 'asm' operand has impossible constraints
mmxfrag.i:16: error: 'asm' operand has impossible constraints

note both -fforce-addr and -fPIC are necessary to produce the problem.


-- 
           Summary: [4.3 Regression] can't find a register in class
                    'GENERAL_REGS' while reloading 'asm'
           Product: gcc
           Version: 4.3.0
            Status: UNCONFIRMED
          Keywords: rejects-valid
          Severity: normal
          Priority: P3
         Component: middle-end
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: rguenth at gcc dot gnu dot org
GCC target triplet: i?86-*-*


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

[Bug middle-end/33713] [4.3 Regression] can't find a register in class 'GENERAL_REGS' while reloading 'asm'

[matz at gcc dot gnu dot org]

------- Comment #1 from matz at gcc dot gnu dot org  2007-10-09 16:50 -------
In 4.2.x CSE1 will already fold back the force-addr insns into the users,
i.e. the asm insn will not even see different registers in those 'm' operands
as address but simply (argp + <something>).  trunk somehow manages to _not_
propagate these force-addr results into the asm, and that then creates this
problem, because that asm then suddenly has seven different pseudo regs as
input :-/


-- 


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

[Bug middle-end/33713] [4.3 Regression] can't find a register in class 'GENERAL_REGS' while reloading 'asm'

[matz at gcc dot gnu dot org]

------- Comment #2 from matz at gcc dot gnu dot org  2007-10-09 17:41 -------
I think this happens because gcc 4.3.x doesn't have fold_rtx_mem() and friends
anymore since the introduction of fwprop.c.  It formerly used to also lookup
the address of the given MEMs, and hence perform this substitution.
Now nothing looks at the address anymore (in cse).  I don't yet know why
fwprop doesn't do that.  Paolo?


-- 

matz at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bonzini at gnu dot org


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

[Bug middle-end/33713] [4.3 Regression] can't find a register in class 'GENERAL_REGS' while reloading 'asm'

[bonzini at gnu dot org]

------- Comment #3 from bonzini at gnu dot org  2007-10-10 06:44 -------
forward_propagate_and_simplify only propagates into single-set insns.  In
principle there's nothing to forbid working on other insns (it already does it
for forward_propagate_subreg), but it is made harder because df does not record
the toplevel SET.

If a brave soul modifies df-scan.c to store that, it will be very easy to fix
this bug. In struct df_ref, "type" only requires an 8-bit bitfield.  This way,
even without increasing memory consumption, there is room for an 8-bit index
into the parallel, or -1 if the PATTERN is a SET itself.


-- 

bonzini at gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |zadeck at gcc dot gnu dot
                   |                            |org
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2007-10-10 06:44:42
               date|                            |


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

[Bug middle-end/33713] [4.3 Regression] can't find a register in class 'GENERAL_REGS' while reloading 'asm'

[pinskia at gcc dot gnu dot org]

------- Comment #4 from pinskia at gcc dot gnu dot org  2007-10-22 08:36 -------
Does this still happen?


-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |pinskia at gcc dot gnu dot
                   |                            |org
   Target Milestone|---                         |4.3.0


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

[Bug middle-end/33713] [4.3 Regression] can't find a register in class 'GENERAL_REGS' while reloading 'asm'

[rguenth at gcc dot gnu dot org]

------- Comment #5 from rguenth at gcc dot gnu dot org  2007-10-22 08:49 -------
Still happens with r129547.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|2007-10-10 06:44:42         |2007-10-22 08:49:32
               date|                            |


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

[Bug middle-end/33713] [4.3 Regression] can't find a register in class 'GENERAL_REGS' while reloading 'asm'

[mmitchel at gcc dot gnu dot org]

-- 

mmitchel at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1


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

[Bug middle-end/33713] [4.3 Regression] can't find a register in class 'GENERAL_REGS' while reloading 'asm'

[bonzini at gnu dot org]

------- Comment #6 from bonzini at gnu dot org  2007-11-06 17:05 -------
I think P1 is a little too much since this requires -fforce-addr.

Anyway, here are my findings and thoughts:

1) reduced testcase:

void oc_frag_recon_inter2_mmx(unsigned char *_dst,int _dst_ystride,
 const unsigned char *_src1,int _src1_ystride,const unsigned char *_src2,
 int _src2_ystride,const int *_residue)
{
  long a;
  __asm__ __volatile__(
   "# %[src2] %[src1_ystride] %[src1] %[a] %[src2_ystride] \n\t"
   "# %[residue] %[dst_ystride] %[dst]\n\t"
   :[a]"=&a"(a),[dst]"+r"(_dst),[residue]"+r"(_residue),
    [src1]"+r"(_src1),[src2]"+r"(_src2)
   :[dst_ystride]"m"((long)_dst_ystride),
    [src1_ystride]"m"((long)_src1_ystride),
    [src2_ystride]"m"((long)_src2_ystride)
 );
}

Note that the testcase is *relying on GCC to undo flag_force_addr!*  In fact,
the author used a "m" constraint exactly because they knew they would run out
of registers: on a less starved machine, one would have used "r"!

This makes me wonder if we shouldn't kill -fforce-addr just like we disposed of
-fforce-mem.  Let's go on anyway and try to fix it.

2) one of the problems is that at -O we do not run fwprop.  We probably want
to.

3) Here is a hack that I thought would fix it.

Index: ../../peak-gcc-src/gcc/stmt.c
===================================================================
--- ../../peak-gcc-src/gcc/stmt.c       (revision 129768)
+++ ../../peak-gcc-src/gcc/stmt.c       (working copy)
@@ -660,6 +660,7 @@ expand_asm_operands (tree string, tree o
   const char **constraints
     = alloca ((noutputs + ninputs) * sizeof (const char *));
   int old_generating_concat_p = generating_concat_p;
+  int save_force_addr = flag_force_addr;

   /* An ASM with no outputs needs to be treated as volatile, for now.  */
   if (noutputs == 0)
@@ -780,6 +781,7 @@ expand_asm_operands (tree string, tree o
   /* Second pass evaluates arguments.  */

   ninout = 0;
+  flag_force_addr = false;
   for (i = 0, tail = outputs; tail; tail = TREE_CHAIN (tail), i++)
     {
       tree val = TREE_VALUE (tail);
@@ -1072,6 +1074,8 @@ expand_asm_operands (tree string, tree o
       emit_insn (body);
     }

+  flag_force_addr = save_force_addr;
+
   /* For any outputs that needed reloading into registers, spill them
      back to where they belong.  */
   for (i = 0; i < noutputs; ++i)

The idea is that in an asm, the author already has full control of memory
modes. If they want to use a simple one, they can (using "r").  Otherwise, they
don't want -fforce-addr (assuming somebody wants it...).

It doesn't work, actually, because CSE not only does not do the propagation in
4.3; with -fforce-addr, it undoes it!!!  So, here are the three possibilities:

a) temporarily set flag_force_addr to false during CSE (in addition to the
above stmt.c hunks, and enabling fwprop at -O).

b) make flag_force_addr effective only during expansion (in addition to the
above stmt.c hunks, and enabling fwprop at -O).  With tree level optimization,
few RTL passes modify mems (CSE, fwprop), and they do so because of addressing
mode selection.  So none of them should benefit of -fforce-addr, even though
only CSE is affected now.

c) disable flag_force_addr completely (doesn't require anything else, we might
still want to enable fwprop at -O?)

My preference is c, b, a.  Anybody else?


-- 


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

[Bug middle-end/33713] [4.3 Regression] can't find a register in class 'GENERAL_REGS' while reloading 'asm'

[rguenther at suse dot de]

------- Comment #7 from rguenther at suse dot de  2007-11-06 21:38 -------
Subject: Re:  [4.3 Regression] can't find a register
 in class 'GENERAL_REGS' while reloading 'asm'

On Tue, 6 Nov 2007, bonzini at gnu dot org wrote:

> I think P1 is a little too much since this requires -fforce-addr.

I agree.  This should be at most a P2.

> c) disable flag_force_addr completely (doesn't require anything else, we might
> still want to enable fwprop at -O?)
> 
> My preference is c, b, a.  Anybody else?

c).  I don't see a reason to not enable fwprop at -O, but I guess in this
late stage I'd avoid it.

Richard.


-- 


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

[Bug middle-end/33713] [4.3 Regression] can't find a register in class 'GENERAL_REGS' while reloading 'asm'

[matz at gcc dot gnu dot org]

------- Comment #8 from matz at gcc dot gnu dot org  2007-11-07 16:45 -------
When I analyzed this for the first time and finally found the root cause
my immediate reaction was "huh? we still have -fforce-addr?" .  So, I also
wanted to get rid of it.  But Richard had some reservations at that time
suspecting that it still has some performance implications sometimes.
But, I mean, nobody ever uses -fforce-addr consciously anymore.  It's in
libtheoras Makefile just because some ancient version of gcc produced better
code with it, sometimes, perhaps.

So if we can agree to dump -fforce-addr: yes, please.


-- 


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

[Bug middle-end/33713] [4.3 Regression] can't find a register in class 'GENERAL_REGS' while reloading 'asm'

[ubizjak at gmail dot com]

------- Comment #9 from ubizjak at gmail dot com  2007-11-07 18:56 -------
(In reply to comment #8)

> So if we can agree to dump -fforce-addr: yes, please.

Sometimes -fforce-addr produces faster code, as claimed in
http://gcc.gnu.org/ml/fortran/2007-10/msg00048.html


-- 


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

[Bug middle-end/33713] [4.3 Regression] can't find a register in class 'GENERAL_REGS' while reloading 'asm'

[paolo dot bonzini at lu dot unisi dot ch]

------- Comment #10 from paolo dot bonzini at lu dot unisi dot ch  2007-11-08 16:32 -------
Subject: Re:  [4.3 Regression] can't find a register
 in class 'GENERAL_REGS' while reloading 'asm'

ubizjak at gmail dot com wrote:
> ------- Comment #9 from ubizjak at gmail dot com  2007-11-07 18:56 -------
> (In reply to comment #8)
> 
>> So if we can agree to dump -fforce-addr: yes, please.
> 
> Sometimes -fforce-addr produces faster code, as claimed in
> http://gcc.gnu.org/ml/fortran/2007-10/msg00048.html

My SPEC2000 run has not finished, but the results so far give a 
different overall picture:

+gzip: 156 -> 167 (without -fforce-addr -> with -fforce-addr)
+vpr: 165 -> 168
+gcc: 68 -> 70
  mcf: 167 -> 167
  crafty: 95 -> 95
  parser: 203 -> 204
+perlbmk: 118 -> 124
+gap: 74 -> 77
-vortex: 142 -> 136
+bzip2: 153 -> 156
+twolf: 223 -> 236

  wupwise: 119 -> 120
-swim: 180 -> 175
  mgrid: 249 -> 251
  applu: 193 -> 194
+mesa: 179 -> 182
+galgel: 145 -> 149
  art: 426 -> 427
-equake: 86 -> 79
+facerec: 190 -> 193

Probably it would be better to spend time distilling a simple testcase 
from equake, which is a relatively small (1500 lines) C program, so that 
the benefit is there with regular -O2.


-- 


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

[Bug middle-end/33713] [4.3 Regression] can't find a register in class 'GENERAL_REGS' while reloading 'asm'

[bonzini at gnu dot org]

------- Comment #11 from bonzini at gnu dot org  2007-11-08 19:56 -------
-fforce-addr for SPECfp is neutral, with big improvements in equake (and a
little on swim+lucas, but the latter has huge fluctuations).

SPECint drops from 1156 to 1130, with clear changes for the worse as
highlighted by my previous comment.

I am also in favor of dropping -fforce-addr.


-- 


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

[Bug middle-end/33713] [4.3 Regression] can't find a register in class 'GENERAL_REGS' while reloading 'asm'

[bonzini at gnu dot org]

------- Comment #12 from bonzini at gnu dot org  2007-11-08 19:58 -------
Who prepares the patch? :-)


-- 


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

[Bug middle-end/33713] [4.3 Regression] can't find a register in class 'GENERAL_REGS' while reloading 'asm'

[steven at gcc dot gnu dot org]

------- Comment #13 from steven at gcc dot gnu dot org  2007-11-10 10:46 -------
Created an attachment (id=14523)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=14523&action=view)
Nukes -fforce-addr


-- 


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

[Bug middle-end/33713] [4.3 Regression] can't find a register in class 'GENERAL_REGS' while reloading 'asm'

[bonzini at gnu dot org]

------- Comment #14 from bonzini at gnu dot org  2007-11-10 13:04 -------
I reviewed the patch and it seems ok except that the option should be kept
undocumented for 4.3.

  fforce-addr
- Common Report Var(flag_force_addr) Optimization
- Copy memory address constants into registers before use
+ Common
+ Does nothing.  Preserved for backward compatibility.


-- 


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

[Bug middle-end/33713] [4.3 Regression] can't find a register in class 'GENERAL_REGS' while reloading 'asm'

[steven at gcc dot gnu dot org]

------- Comment #15 from steven at gcc dot gnu dot org  2007-11-19 08:54 -------
Cute little patch of comment #13 looking for foster parent.


-- 


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

[Bug middle-end/33713] [4.3 Regression] can't find a register in class 'GENERAL_REGS' while reloading 'asm'

[bonzini at gnu dot org]

------- Comment #16 from bonzini at gnu dot org  2007-11-19 09:45 -------
Steven, post it to gcc-patches and I'll be happy to commit it as soon as it is
approved.


-- 


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

[Bug middle-end/33713] [4.3 Regression] can't find a register in class 'GENERAL_REGS' while reloading 'asm'

[aldyh at gcc dot gnu dot org]

------- Comment #17 from aldyh at gcc dot gnu dot org  2007-11-29 11:56 -------
I am testing this patch, and will post to gcc-patches when they finish.


-- 


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

[Bug middle-end/33713] [4.3 Regression] can't find a register in class 'GENERAL_REGS' while reloading 'asm'

[aldyh at gcc dot gnu dot org]

------- Comment #18 from aldyh at gcc dot gnu dot org  2007-12-03 23:21 -------
http://gcc.gnu.org/ml/gcc-patches/2007-11/msg01651.html


-- 

aldyh at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED


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