linux-sparse.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
To: Ramsay Jones <ramsay@ramsayjones.plus.com>
Cc: linux-sparse@vger.kernel.org
Subject: Re: [PATCH v2 3/3] add more testcases for AND/OR simplification
Date: Tue, 8 Sep 2020 00:21:22 +0200	[thread overview]
Message-ID: <20200907222122.ruonzxhhfjk6dix2@ltop.local> (raw)
In-Reply-To: <7ead3fd3-34f4-f5d4-21fa-c7937fcab5fe@ramsayjones.plus.com>

On Mon, Sep 07, 2020 at 01:15:18AM +0100, Ramsay Jones wrote:
> On 06/09/2020 22:16, Luc Van Oostenryck wrote:
> > Add a few testcases showing the effectiveness of these
> > simplifications and to catch possible future regressions.
> > 
> 
> Sorry, I had to step away from the keyboard for a couple
> of hours ...

No, problem, of course.
 
> > diff --git a/validation/optim/and-lsr-or-shl0.c b/validation/optim/and-lsr-or-shl0.c
> > new file mode 100644
> > index 000000000000..46ab1bde5249
> > --- /dev/null
> > +++ b/validation/optim/and-lsr-or-shl0.c
> > @@ -0,0 +1,13 @@
> > +// =>	0
> > +unsigned int and_lsr_or_shl0(unsigned int a, unsigned int b)
> > +{
> > +	return ((a | b << 12) >> 12) & 0xfff00000;
> > +}
> > +
> > +/*
> > + * check-name: and-lsr-or-shl0
> > + * check-command: test-linearize -Wno-decl $file
> > + * check-known-to-fail
> > + *
> > + * check-output-excludes: shl\\.
> 
> Why not something like:
>   * check-output-contains: ret.32 *\\$0

Yes, at the end this check is the only thing that matters. And since
it's all pure expressions, if there is a return with a constant,
no other instructions can possibly be present.

>   * check-output-excludes: shl\\.
>   * check-output-excludes: or\\.
>   * check-output-excludes: lsr\\.
>   * check-output-excludes: and\\.

But these tests were written (more than 2 years ago, so I forgot the
details about them) for very specific steps in the simplification
phase, most probably aiming bitfields (hence the constant shifts &
masks). In this case it was for a simplification that removed the '<<'.

> > diff --git a/validation/optim/and-lsr-or-shl1.c b/validation/optim/and-lsr-or-shl1.c
> > new file mode 100644
> > index 000000000000..22fee362b16b
> > --- /dev/null
> > +++ b/validation/optim/and-lsr-or-shl1.c
> > @@ -0,0 +1,13 @@
> > +// =>	(((a | b << 12) >> 12)
> > +unsigned int and_lsr_or_shl1(unsigned int a, unsigned int b)
> > +{
> > +	return ((a | b << 12) >> 12) & 0x000fffff;
> > +}
> > +
> > +/*
> > + * check-name: and-lsr-or-shl1
> > + * check-command: test-linearize -Wno-decl $file
> > + * check-known-to-fail
> > + *
> > + * check-output-excludes: shl\\.
> 
> Hmm, this should be ': and\\.' right?

My intention was most certainly to test the shl but my comment
here above is wrong. It should have been:
	//	((a | (b << 12)) >> 12) & 0x000fffff
	// ->	((a >> S) | ((b << S) >> S)) & 0x000fffff
	// ->	((a >> S) | (b & 0x000fffff)) & 0x000fffff
	// =>	((a >> S) | b) & 0x000fffff
	// or	(a >> S) | (b & 0x000fffff)

> > diff --git a/validation/optim/and-shl-or-lsr0.c b/validation/optim/and-shl-or-lsr0.c
> > new file mode 100644
> > index 000000000000..f2a7cc631258
> > --- /dev/null
> > +++ b/validation/optim/and-shl-or-lsr0.c
> > @@ -0,0 +1,13 @@
> 
> Hmm, I can't see the optimization, just ...
> 
> > +unsigned and_shl_or_lsr0(unsigned a, unsigned b)
> > +{
> > +	return ((a | (b >> 12)) << 12) & 0xfff00000;
> 
> ->((a << 12) | ((b >> 12) << 12)) & 0xfff00000
> ->((a << 12) | b) & 0xfff00000
> so that ...
> > +}
> > +
> > +/*
> > + * check-name: and-shl-or-lsr0
> > + * check-command: test-linearize -Wno-decl $file
> > + * check-known-to-fail
> > + *
> > + * check-output-ignore
> > + * check-output-excludes: or\\.
> 
> ... this wouldn't be correct. puzzled! :(

Indeed, I probably meant 0x00000fff instead of 0xfff00000
	//	((a | (b >> S)) << S) & 0x00000fff
	// ->	((a << S) | ((b >> S) << S)) & 0x00000fff
	// ->	((a << S) | (b & 0xfffff000)) & 0x00000fff
	// ->	(a << S) & 0x00000fff
and then:
	// =>	0

> > diff --git a/validation/optim/lsr-or-lsr0.c b/validation/optim/lsr-or-lsr0.c
> > new file mode 100644
> > index 000000000000..aad4aa7fda56
> > --- /dev/null
> > +++ b/validation/optim/lsr-or-lsr0.c
> > @@ -0,0 +1,22 @@
> > +#define	S	12
> > +
> > +//	((x >> S') | y) >> S;
> > +// ->	((x >> S' >> S) | (y >> S)
> 
> s/((x/(x/
> 
> > +// ->	((x >> 32) | (y >> S)
> 
> s/((x/(x/
> 
> > +// =>	(y >> S)
> > +
> > +int lsr_or_lsr0(unsigned int x, unsigned int b)
> > +{
> > +	return ((x >> (32 - S)) | b) >> S;
> > +}
> > +
> > +/*
> > + * check-name: lsr-or-lsr0
> > + * check-command: test-linearize -Wno-decl $file
> > + * check-known-to-fail
> > + *
> > + * check-output-ignore
> > + * check-output-pattern(1): lsr\\.
> > + * check-output-excludes: and\\.
> 
> why would an 'and' be here anyway?

Because each shift has a implicit mask associated with it:
	(x >> S) == ((x & 0xffffffff) >> S) == (x >> S) & 0x000fffff
In some simplifications I made, it becomes an explicit mask and
sometimes there was a left-over. But yes, it's not very interesting here.

Thanks for noticing all this. I'll sort & reorganize them.

-- Luc

  reply	other threads:[~2020-09-07 22:21 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-06 21:16 [PATCH v2 0/3] fix & extend mask related testcases Luc Van Oostenryck
2020-09-06 21:16 ` [PATCH v2 1/3] optim: fix some testcases related to bitfield manipulation Luc Van Oostenryck
2020-09-06 21:16 ` [PATCH v2 2/3] add more testcases for existing AND/OR simplifications Luc Van Oostenryck
2020-09-06 21:54   ` Ramsay Jones
2020-09-07 22:28     ` Luc Van Oostenryck
2020-09-06 21:16 ` [PATCH v2 3/3] add more testcases for AND/OR simplification Luc Van Oostenryck
2020-09-07  0:15   ` Ramsay Jones
2020-09-07 22:21     ` Luc Van Oostenryck [this message]
2020-09-08 15:39       ` Ramsay Jones

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200907222122.ruonzxhhfjk6dix2@ltop.local \
    --to=luc.vanoostenryck@gmail.com \
    --cc=linux-sparse@vger.kernel.org \
    --cc=ramsay@ramsayjones.plus.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).