diff mbox series

[1/2] analyzer: Remove check of unsigned_char in maybe_undo_optimize_bit_field_compare.

Message ID 20231210195650.1772459-2-quic_apinski@quicinc.com
State Accepted
Headers
Series Fix PR 111972: Missed vectorization due to phiopt changes |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Andrew Pinski (QUIC) Dec. 10, 2023, 7:56 p.m. UTC 
  From: Andrew Pinski <apinski@marvell.com>

The check for the type seems unnecessary and gets in the way sometimes.
Also with a patch I am working on for match.pd, it causes a failure to happen.
Before my patch the IR was:
  _1 = BIT_FIELD_REF <s, 8, 16>;
  _2 = _1 & 1;
  _3 = _2 != 0;
  _4 = (int) _3;
  __analyzer_eval (_4);

Where _2 was an unsigned char type.
And After my patch we have:
  _1 = BIT_FIELD_REF <s, 8, 16>;
  _2 = (int) _1;
  _3 = _2 & 1;
  __analyzer_eval (_3);

But in this case, the BIT_AND_EXPR is in an int type.

OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.

gcc/analyzer/ChangeLog:

	* region-model-manager.cc (maybe_undo_optimize_bit_field_compare): Remove
	the check for type being unsigned_char_type_node.
---
 gcc/analyzer/region-model-manager.cc | 3 ---
 1 file changed, 3 deletions(-)

Comments

Richard Biener Dec. 11, 2023, 8:04 a.m. UTC | #1 
On Sun, Dec 10, 2023 at 8:57 PM Andrew Pinski <quic_apinski@quicinc.com> wrote:
>
> From: Andrew Pinski <apinski@marvell.com>
>
> The check for the type seems unnecessary and gets in the way sometimes.
> Also with a patch I am working on for match.pd, it causes a failure to happen.
> Before my patch the IR was:
>   _1 = BIT_FIELD_REF <s, 8, 16>;
>   _2 = _1 & 1;
>   _3 = _2 != 0;
>   _4 = (int) _3;
>   __analyzer_eval (_4);
>
> Where _2 was an unsigned char type.
> And After my patch we have:
>   _1 = BIT_FIELD_REF <s, 8, 16>;
>   _2 = (int) _1;
>   _3 = _2 & 1;
>   __analyzer_eval (_3);
>
> But in this case, the BIT_AND_EXPR is in an int type.
>
> OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.

OK (hope it's OK that I approve this).

Richard.

> gcc/analyzer/ChangeLog:
>
>         * region-model-manager.cc (maybe_undo_optimize_bit_field_compare): Remove
>         the check for type being unsigned_char_type_node.
> ---
>  gcc/analyzer/region-model-manager.cc | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/gcc/analyzer/region-model-manager.cc b/gcc/analyzer/region-model-manager.cc
> index b631bcb04d0..26c34e38875 100644
> --- a/gcc/analyzer/region-model-manager.cc
> +++ b/gcc/analyzer/region-model-manager.cc
> @@ -596,9 +596,6 @@ maybe_undo_optimize_bit_field_compare (tree type,
>                                        tree cst,
>                                        const svalue *arg1)
>  {
> -  if (type != unsigned_char_type_node)
> -    return NULL;
> -
>    const binding_map &map = compound_sval->get_map ();
>    unsigned HOST_WIDE_INT mask = TREE_INT_CST_LOW (cst);
>    /* If "mask" is a contiguous range of set bits, see if the
> --
> 2.39.3
>
David Malcolm Dec. 11, 2023, 2:18 p.m. UTC | #2 
On Mon, 2023-12-11 at 09:04 +0100, Richard Biener wrote:
> On Sun, Dec 10, 2023 at 8:57 PM Andrew Pinski
> <quic_apinski@quicinc.com> wrote:
> > 
> > From: Andrew Pinski <apinski@marvell.com>
> > 
> > The check for the type seems unnecessary and gets in the way
> > sometimes.
> > Also with a patch I am working on for match.pd, it causes a failure
> > to happen.
> > Before my patch the IR was:
> >   _1 = BIT_FIELD_REF <s, 8, 16>;
> >   _2 = _1 & 1;
> >   _3 = _2 != 0;
> >   _4 = (int) _3;
> >   __analyzer_eval (_4);
> > 
> > Where _2 was an unsigned char type.
> > And After my patch we have:
> >   _1 = BIT_FIELD_REF <s, 8, 16>;
> >   _2 = (int) _1;
> >   _3 = _2 & 1;
> >   __analyzer_eval (_3);
> > 
> > But in this case, the BIT_AND_EXPR is in an int type.
> > 
> > OK? Bootstrapped and tested on x86_64-linux-gnu with no
> > regressions.

Yes...

> 
> OK (hope it's OK that I approve this).

...and yes.

Dave
diff mbox series

Patch

diff --git a/gcc/analyzer/region-model-manager.cc b/gcc/analyzer/region-model-manager.cc
index b631bcb04d0..26c34e38875 100644
--- a/gcc/analyzer/region-model-manager.cc
+++ b/gcc/analyzer/region-model-manager.cc
@@ -596,9 +596,6 @@  maybe_undo_optimize_bit_field_compare (tree type,
 				       tree cst,
 				       const svalue *arg1)
 {
-  if (type != unsigned_char_type_node)
-    return NULL;
-
   const binding_map &map = compound_sval->get_map ();
   unsigned HOST_WIDE_INT mask = TREE_INT_CST_LOW (cst);
   /* If "mask" is a contiguous range of set bits, see if the