Re: [Qemu-devel] [PATCH 0/4] tcg/optimize: fixes and improvements

[Aurelien Jarno <aurelien@aurel32.net>]

On Tue, Sep 03, 2013 at 09:21:06AM +0200, Paolo Bonzini wrote:
> Il 03/09/2013 08:27, Aurelien Jarno ha scritto:
> > This patchset first fixes known-zero bits optimization so that it is
> > actually used, and does some further optimizations for 32-bit ops and
> > unsigned loads.
> > 
> > Aurelien Jarno (4):
> >   tcg/optimize: fix know-zero bits optimization
> >   tcg/optimize: fix known-zero bits for right shift ops
> >   tcg/optimize: improve known-zero bits for 32-bit ops
> >   tcg/optimize: add known-zero bits compute for load ops
> > 
> >  tcg/optimize.c |   48 +++++++++++++++++++++++++++++++++++++++++++-----
> >  1 file changed, 43 insertions(+), 5 deletions(-)
> > 
> 
> Commit message 1 is a bit misleading, because the optimization still
> works for quite a few cases involving constant and copy propagation.
> However, I had the same patch in my queue, so I can't deny that there is
> a problem. :)

I have just checked, and it does indeed work for a few cases involving
constants. That said, it doesn't change the resulting TCG code, as these
cases were already handled by some other optimizations.

That let me ask a question, about why the bit propagation has been added
in the middle of other optimizations, and not for example immediately
after swapping commutative ops or just before the constant folding.

> Two questions:
> 
> 1) should patch 2 be CCed to qemu-stable?

I considered it was not a problem given I thought the optimization was
basically disabled. I'll fix that in the new version.

> 2) should patches 1 and 2 be inverted to avoid triggering bugs?

Good idea.

Thanks for the review, I'll send an updated version.

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net