Re: [patch] PR33600, fallout from pr33552 fix

[Michael Matz <matz at suse dot de>]

Hi,

On Mon, 1 Oct 2007, Jakub Jelinek wrote:

> On Mon, Oct 01, 2007 at 04:50:59AM +0200, Michael Matz wrote:
> > a thinko caused this.  Funny that it didn't trigger on any of the four 
> > test platforms, but on a kernel compile :-/
> > 
> > Will checkin when bootstrap and regtesting succeeds on i686.
> 
> This isn't sufficient.

It is sufficient but not optimal.

> The optimization already in the checking phase
> should bail out if there is another matching constraint pair, which uses
> the same REG as we want to change.
> ...
> then we don't want to replace all "n" inputs to "x", because it
> really is not an optimization - while it improves the situation
> for one pair, it pessimizes the other pair.

I did notice the problem, but decided to not solve it for the following 
reason: The problem is harmless (in the sense that it first fixes up one 
pair, and then fixes up the other pair, i.e. emits two moves which aren't 
really necessary, as the final instruction is just as bad as the initial 
one).  And as you say really solving this would mean some much more 
complex solution, without large gain.  No large gain, because even those 
two superfluous moves will be coalesced away by the reg allocator.

I also considered some easier work-around along your lines.  The problem 
with that is that it might make this pass not do what it's supposed to do.  
If I were to not fix up one constraint pair simply because an output is 
already the same as an input we might miss fixing up a situation which 
really requires it, namely in the case that that other output did _not_ 
have a matching problematic pair.

For instance with that additional cutoff the following testcase would not 
compile anymore with -O2 -fPIC on x86 (it does with my patch):

void use (int);
void f (int a, int b, int c, int d, int e, int f)
{
  int orig = a;
  __asm__  __volatile__ (""
           : "=r" (a), "=mr" (a)
           : "0"   (a), "1" (a), "r"(c), "r"(d), "r"(e), "r"(f)  );
  use (orig);
}

So, I rather leave in the harmless short-living code pessimization which 
will be magically fixed by coalescing, instead of reducing the number of 
cases where this transformation applies.  I hold off committing the patch 
for now nevertheless to hear your opinion about the above.


Ciao,
Michael.