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
next prev parent 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).