| Message ID | 20200906211646.58946-4-luc.vanoostenryck@gmail.com (mailing list archive) |
|---|---|
| State | Rejected, archived |
| Headers | show |
| Series | fix & extend mask related testcases | expand |
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 ... > Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com> > --- > validation/optim/and-lsr-or-shl0.c | 13 +++++++++++++ > validation/optim/and-lsr-or-shl1.c | 13 +++++++++++++ > validation/optim/and-shl-or-lsr0.c | 13 +++++++++++++ > validation/optim/lsr-or-lsr0.c | 22 ++++++++++++++++++++++ > validation/optim/trunc-or-shl0.c | 22 ++++++++++++++++++++++ > 5 files changed, 83 insertions(+) > create mode 100644 validation/optim/and-lsr-or-shl0.c > create mode 100644 validation/optim/and-lsr-or-shl1.c > create mode 100644 validation/optim/and-shl-or-lsr0.c > create mode 100644 validation/optim/lsr-or-lsr0.c > create mode 100644 validation/optim/trunc-or-shl0.c > > 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 * check-output-excludes: shl\\. * check-output-excludes: or\\. * check-output-excludes: lsr\\. * check-output-excludes: and\\. > + */ > 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? > + */ > 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! :( > + */ > 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? > + * check-output-excludes: or\\. > + */ > diff --git a/validation/optim/trunc-or-shl0.c b/validation/optim/trunc-or-shl0.c > new file mode 100644 > index 000000000000..ab92aca1b711 > --- /dev/null > +++ b/validation/optim/trunc-or-shl0.c > @@ -0,0 +1,22 @@ > +// => TRUNC(b, 8) > +char trunc_or_shl0a(unsigned a, unsigned b) > +{ > + return (a << 8) | b; > +} > + > +// => TRUNC(a, 8) > +char trunc_or_shl0b(unsigned a, unsigned b) > +{ > + return a | (b << 8); > +} > + > +/* > + * check-name: trunc-or-shl0 > + * check-command: test-linearize -Wno-decl $file > + * check-known-to-fail > + * > + * check-output-ignore > + * check-output-excludes: or\\. > + * check-output-excludes: shl\\. > + * check-output-pattern(2): %arg OK, good. > + */ > ATB, Ramsay Jones
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
On 07/09/2020 23:21, Luc Van Oostenryck wrote: >>> 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. :-D OK, so let's imagine there is a bug (perhaps an off-by-one, say) that results in some IR _not_ being removed properly, will this test catch that? Does it matter? why not? Given that you don't provide an '*.expected' file, adding the following to the test won't guarantee to catch that bug either, but would hopefully increase the odds. (this assumes that the pre-optimization pass IR contains 'shl', 'or', 'lsr' and 'and', of course). > >> * 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 '<<'.> [snip] >>> 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. ;-P So, imagine you didn't know the inner workings of the optimization code, looking at the test text alone, would you understand why the test asserts what it does? (The input IR to the algorithm doesn't have an 'and', right?) [Similar comment about other email on patch #2, the input IR doesn't have an 'lsr', right?] OK, I promise not to comment further on this series! ;-) ATB, Ramsay Jones
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\\. + */ 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\\. + */ 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 @@ +unsigned and_shl_or_lsr0(unsigned a, unsigned b) +{ + return ((a | (b >> 12)) << 12) & 0xfff00000; +} + +/* + * check-name: and-shl-or-lsr0 + * check-command: test-linearize -Wno-decl $file + * check-known-to-fail + * + * check-output-ignore + * check-output-excludes: or\\. + */ 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) +// -> ((x >> 32) | (y >> S) +// => (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\\. + * check-output-excludes: or\\. + */ diff --git a/validation/optim/trunc-or-shl0.c b/validation/optim/trunc-or-shl0.c new file mode 100644 index 000000000000..ab92aca1b711 --- /dev/null +++ b/validation/optim/trunc-or-shl0.c @@ -0,0 +1,22 @@ +// => TRUNC(b, 8) +char trunc_or_shl0a(unsigned a, unsigned b) +{ + return (a << 8) | b; +} + +// => TRUNC(a, 8) +char trunc_or_shl0b(unsigned a, unsigned b) +{ + return a | (b << 8); +} + +/* + * check-name: trunc-or-shl0 + * check-command: test-linearize -Wno-decl $file + * check-known-to-fail + * + * check-output-ignore + * check-output-excludes: or\\. + * check-output-excludes: shl\\. + * check-output-pattern(2): %arg + */
Add a few testcases showing the effectiveness of these simplifications and to catch possible future regressions. Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com> --- validation/optim/and-lsr-or-shl0.c | 13 +++++++++++++ validation/optim/and-lsr-or-shl1.c | 13 +++++++++++++ validation/optim/and-shl-or-lsr0.c | 13 +++++++++++++ validation/optim/lsr-or-lsr0.c | 22 ++++++++++++++++++++++ validation/optim/trunc-or-shl0.c | 22 ++++++++++++++++++++++ 5 files changed, 83 insertions(+) create mode 100644 validation/optim/and-lsr-or-shl0.c create mode 100644 validation/optim/and-lsr-or-shl1.c create mode 100644 validation/optim/and-shl-or-lsr0.c create mode 100644 validation/optim/lsr-or-lsr0.c create mode 100644 validation/optim/trunc-or-shl0.c