Hi!
On Thu, Jan 20, 2022 at 01:46:48PM -0500, David Edelsohn wrote:
> On Thu, Jan 20, 2022 at 2:36 AM HAO CHEN GUI <guih...@linux.ibm.com> wrote:
> > This patch adds a combine pattern for "CA minus one". As CA only has two
> > values (0 or 1), we could convert following pattern
> > (sign_extend:DI (plus:SI (reg:SI 98 ca)
> > (const_int -1 [0xffffffffffffffff]))))
> > to
> > (plus:DI (reg:DI 98 ca)
> > (const_int -1 [0xffffffffffffffff])))
> > With this patch, one unnecessary sign extend is eliminated.
> >
> > Bootstrapped and tested on powerpc64-linux BE and LE with no regressions.
> > Is this okay for trunk? Any recommendations? Thanks a lot.
There are ten gazillion similar things we could make extra backend
patterns for, and we still would not cover a majority of cases.
If instead we got some generic way to handle this we could cover many
more cases, for much less effort.
We need both widening modes from SI to DI, amd narrowing modes from DI
to SI. Both are useful in certain cases; it is not like using wider
modes is always better, in some cases narrower modes is better (in cases
where we can let the generated code then generate whatever bits in the
high half of the word, for example; a typical example is addition in an
unsigned int).
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/powerpc/pr95737.c
> > @@ -0,0 +1,10 @@
> > +/* PR target/95737 */
> > +/* { dg-do compile { target lp64 } } */
> > +/* { dg-options "-O2 -mdejagnu-cpu=power8" } */
>
> Why does the testcase force power8? This testcase is not specific to
> Power8 or later.
Yes, and we should generate the same code on older machines.
> > +/* { dg-final { scan-assembler-not {\mextsw\M} } } */
> > +
> > +
> > +unsigned long long negativeLessThan (unsigned long long a, unsigned long
> > long b)
> > +{
> > + return -(a < b);
> > +}
>
> If you're only testing for lp64, the testcase could use "long" instead
> of "long long".
The testcase really needs "powerpc64", if that would mean "test if
-mpowerpc64 is (implicitly) used". But that is not what it currently
means (it is something akin to "powerpc64_hw", instead).
So we test lp64, which is set if and only if -m64 was used. It is
reasonable coverage, no one cares much for -m32 -mpowerpc64 .
Segher
