Re: [PATCH 1/3] add more testcases for existing AND/OR simplifications

[Luc Van Oostenryck <luc.vanoostenryck@gmail.com>]

Hi,

On Sun, Sep 06, 2020 at 05:12:40PM +0100, Ramsay Jones wrote:
> On 06/09/2020 13:40, Luc Van Oostenryck wrote:
> > Add a few more testcases to catch possible future regressions.
> > 
> > Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> > ---
> >  validation/optim/and-shl-or-and0.c  | 13 +++++++++++++
> >  validation/optim/lsr-or-and0.c      | 23 +++++++++++++++++++++++
> >  validation/optim/shl-or-constant0.c | 12 ++++++++++++
> >  validation/optim/shl-or-constant1.c | 12 ++++++++++++
> >  validation/optim/shl-or-constant2.c | 12 ++++++++++++
> >  5 files changed, 72 insertions(+)
> >  create mode 100644 validation/optim/and-shl-or-and0.c
> >  create mode 100644 validation/optim/lsr-or-and0.c
> >  create mode 100644 validation/optim/shl-or-constant0.c
> >  create mode 100644 validation/optim/shl-or-constant1.c
> >  create mode 100644 validation/optim/shl-or-constant2.c
> > 
> > diff --git a/validation/optim/and-shl-or-and0.c b/validation/optim/and-shl-or-and0.c
> > new file mode 100644
> > index 000000000000..ea08d2622a95
> > --- /dev/null
> > +++ b/validation/optim/and-shl-or-and0.c
> > @@ -0,0 +1,13 @@
> > +unsigned and_shl_or_and0(unsigned a, unsigned b)
> > +{
> > +	return (((a & 0xfff00000) | b) << 12) & 0xfff00000;
> 
> ->(((a & 0xfff00000) << 12) | (b << 12)) & 0xfff00000
> ->(( 0 | (b << 12)) & 0xfff00000
> ->((b << 12)) & 0xfff00000
> > +}
> > +
> > +/*
> > + * check-name: and-shl-or-and0
> > + * check-command: test-linearize -Wno-decl $file
> > + *
> > + * check-output-ignore
> > + * check-output-excludes: or\\.
> > + * check-output-excludes: lsr\\.
> 
> why would there be a right-shift to begin with?
> (maybe add check-output-excludes: %arg1)

I'm not sure. It may be an error in the testcase, maybe a copy-paste
from some other tests, but I think it comes from some simplification
steps involving masks and shift and where a masking operation like
(x & 0xfff00000) is first virtually transformed into ((x >> 20) << 20)
before being simplified away.
Yes, checking the absence of %arg1 is a good idea.

> > + */
> > diff --git a/validation/optim/lsr-or-and0.c b/validation/optim/lsr-or-and0.c
> > new file mode 100644
> > index 000000000000..3c369cb9497e
> > --- /dev/null
> > +++ b/validation/optim/lsr-or-and0.c
> > @@ -0,0 +1,23 @@
> > +#define	S	12
> > +
> > +//	((x & M) | b) >> S;
> > +// ->	((x >> S) & (M >> S)) | (b >> S)
> 
> OK
> 
> > +// 0a:  (M >> S) == 0
> > +// 0b:  (x >> S) == 0
> > +// 0c:  (b >> S) == 0
> 
> I do not understand what these three lines are trying to say! :(

It's just some leftover of personal notes about the 3 opportunities 
of simplifications. It's probably best to remove.

> > +
> > +int lsr_or_and0a(unsigned int x, unsigned int b)
> 
> s/and0a/and0/ - was there an '_and0b' at one time?

Yes, most probably.

> > diff --git a/validation/optim/shl-or-constant2.c b/validation/optim/shl-or-constant2.c
> > new file mode 100644
> > index 000000000000..9dbde3b574d7
> > --- /dev/null
> > +++ b/validation/optim/shl-or-constant2.c
> > @@ -0,0 +1,12 @@
> > +unsigned shl_or_constant1(unsigned a)
> 
> s/_constant1/_constant2/

Yes, it's better so.

Thanks,
-- Luc