Re: [PATCH 1/4 v3][PR 67328] Generate bittests in range checks if possible

[Richard Sandiford]

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.

> +
> +  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.

> +  *mask = wide_int_to_tree (type, stem_mask);
> +  *value = wide_int_to_tree (type, stem);
> +
> +  return true;

Thanks,
Richard