On Thu, 8 Jun 2017, Yuri Gribov wrote:
> On Tue, May 30, 2017 at 7:25 AM, Richard Sandiford
> <richard.sandif...@linaro.org> wrote:
> > Yuri Gribov <tetra2...@gmail.com> writes:
> >> Added special case to build_range_check. Fixed couple of existing
> >> tests where it changed codegen.
> >>
> >> -I
> >>
> >> From b7819f341e2ffa0437be497024f61d0a4e1be588 Mon Sep 17 00:00:00 2001
> >> From: Yury Gribov <tetra2...@gmail.com>
> >> Date: Fri, 26 May 2017 07:49:46 +0100
> >> Subject: [PATCH 1/4] Generate bittests in range checks if possible.
> >>
> >> gcc/testsuite/
> >> 2017-05-26 Yury Gribov <tetra2...@gmail.com>
> >>
> >> * c-c++-common/fold-masked-cmp-1.c: New test.
> >> * c-c++-common/fold-masked-cmp-2.c: New test.
> >> * gcc.dg/pr46309.c: Fix pattern.
> >> * gcc.dg/pr46309-2.c: Fix pattern.
> >>
> >> gcc/
> >> 2017-05-26 Yury Gribov <tetra2...@gmail.com>
> >>
> >> * fold-const.c (maskable_range_p): New function.
> >> (build_range_check): Generate bittests if possible.
> >> ---
> >> gcc/fold-const.c | 41
> >> ++++++++++++++++++++++++-
> >> gcc/testsuite/c-c++-common/fold-masked-cmp-1.c | 41
> >> +++++++++++++++++++++++++
> >> gcc/testsuite/c-c++-common/fold-masked-cmp-2.c | 42
> >> ++++++++++++++++++++++++++
> >> gcc/testsuite/gcc.dg/pr46309-2.c | 2 +-
> >> gcc/testsuite/gcc.dg/pr46309.c | 2 +-
> >> 5 files changed, 125 insertions(+), 3 deletions(-)
> >> create mode 100644 gcc/testsuite/c-c++-common/fold-masked-cmp-1.c
> >> create mode 100644 gcc/testsuite/c-c++-common/fold-masked-cmp-2.c
> >>
> >> diff --git a/gcc/fold-const.c b/gcc/fold-const.c
> >> index efc0b10..c334dc6 100644
> >> --- a/gcc/fold-const.c
> >> +++ b/gcc/fold-const.c
> >> @@ -4745,6 +4745,38 @@ make_range (tree exp, int *pin_p, tree *plow, tree
> >> *phigh,
> >> *pin_p = in_p, *plow = low, *phigh = high;
> >> return exp;
> >> }
> >> +
> >> +/* Returns TRUE is [LOW, HIGH] range check can be optimized to
> >
> > s/is/if/
> >
> >> + a bitwise check i.e. when
> >> + LOW == 0xXX...X00...0
> >> + HIGH == 0xXX...X11...1
> >> + Return corresponding mask in MASK and stem in VALUE. */
> >> +
> >> +static bool
> >> +maskable_range_p (const_tree low, const_tree high, tree type, tree *mask,
> >> tree *value)
> >> +{
> >> + if (TREE_CODE (low) != INTEGER_CST
> >> + || TREE_CODE (high) != INTEGER_CST)
> >> + return false;
> >> +
> >> + widest_int lo = wi::to_widest (low),
> >> + hi = wi::to_widest (high);
> >
> > It shouldn't be necessary to go to widest here, since AIUI both
> > values have the same type.
>
> Done.
>
> >> + widest_int end_mask = lo ^ hi,
> >> + stem_mask = ~end_mask;
> >> + if ((end_mask & (end_mask + 1)) != 0
> >> + || (lo & end_mask) != 0)
> >> + return false;
> >> +
> >> + widest_int stem = lo & stem_mask;
> >> + if (stem != (hi & stem_mask))
> >> + return false;
> >
> > FWIW, I think this is equivalent to:
> >
> > if (lo + (lo & -lo) != hi + 1)
> > return false;
> >
> > but I guess it's a matter of taste whether that's clearer or not.
>
> The change is cure but frankly I'd rather keep original version for
> the sake of readability...
>
> >> + *mask = wide_int_to_tree (type, stem_mask);
> >> + *value = wide_int_to_tree (type, stem);
> >> +
> >> + return true;
>
> I've attached an updated patch (rebased and retested on x64). Ok to commit?
Ok if you move stem_mask computation
+ wide_int end_mask = lo ^ hi,
+ stem_mask = ~end_mask;
after
+ if ((end_mask & (end_mask + 1)) != 0
+ || (lo & end_mask) != 0)
+ return false;
+
Thanks,
Richard.
