[Bug tree-optimization/110554] New: more invalid wide Boolean values

[Bug tree-optimization/110554] New: more invalid wide Boolean values

[kristerw at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110554

            Bug ID: 110554
           Summary: more invalid wide Boolean values
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: kristerw at gcc dot gnu.org
  Target Milestone: ---

The fix for PR 110487 improved the situation, but my tool still finds some
cases where GCC generates invalid <signed-boolean:32> values.

One such case can be seen in gcc.c-torture/compile/pr104499.c:

  typedef int __attribute__((__vector_size__ (8 * sizeof (int)))) V;

  V v;

  void
  foo (void)
  {
    v = ((1 | v) != 1);
  }

Here veclower2 is introducing code

  <signed-boolean:32> _8;
  <signed-boolean:32> _10;
  ...
  gimple_assign <gt_expr, _8, _7, 1, NULL>
  gimple_assign <bit_and_expr, _10, _8, 1, NULL>


More examples of this failure can be seen in gcc.c-torture/compile/pr108237.c
and gcc.c-torture/compile/pr54713-1.c

[Bug tree-optimization/110554] more invalid wide Boolean values

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110554

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |pinskia at gcc dot gnu.org

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
The case here is the match pattern which does:
((type)a)&1 into (type)(a&1).

Will double check.

[Bug tree-optimization/110554] veclowering introduces invalid wide Boolean values

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110554

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |internal-improvement,
                   |                            |wrong-code
            Summary|more invalid wide Boolean   |veclowering introduces
                   |values                      |invalid wide Boolean values
   Last reconfirmed|                            |2023-07-05
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Simplified testcase (that will fail on all targets):
  typedef int __attribute__((__vector_size__ (1 * sizeof (int)))) V;

  V v;

  void
  foo (void)
  {
    v = ((1 | v) != 1);
  }

[Bug tree-optimization/110554] veclowering introduces invalid wide Boolean values

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110554

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
It is this one:
/* Handle cases of two conversions in a row.  */
(for ocvt (convert float fix_trunc)
 (for icvt (convert float)
  (simplify
   (ocvt (icvt@1 @0))
...


    /* A truncation to an unsigned type (a zero-extension) should be
       canonicalized as bitwise and of a mask.  */
    (if (GIMPLE /* PR70366: doing this in GENERIC breaks -Wconversion.  */
         && final_int && inter_int && inside_int
         && final_prec == inside_prec
         && final_prec > inter_prec
         && inter_unsignedp)
     (convert (bit_and @0 { wide_int_to_tree
                              (inside_type,
                               wi::mask (inter_prec, false,
                                         TYPE_PRECISION (inside_type))); })))

[Bug tree-optimization/110554] veclowering introduces invalid wide Boolean values

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110554

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
So originally we have:

signed-boolean:32 _8 = _7 > 1;
_Bool _9 = (_Bool) _8;
(int) _9;

I think if we swap around where the convert was, it will fix the issue.
That is:
     (bit_and (convert @0) { wide_int_to_tree
                              (type,
                               wi::mask (inter_prec, false,
                                         TYPE_PRECISION (type))); })))


But maybe we should check for which is a boolean type.

[Bug tree-optimization/110554] veclowering introduces invalid wide Boolean values

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110554

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Actually the issue is:
signed-boolean:32 _8 = _7 > 1;

That is not [-1,0] but still [0,1] ... (VRP says it is too).

Let me see where that is built.

[Bug tree-optimization/110554] veclowering introduces invalid wide Boolean values

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110554

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |pinskia at gcc dot gnu.org

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Created attachment 55472
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55472&action=edit
Fix veclowering types

So the problem is veclowering should have just use boolean_type_node here for
the comparison.

There is another type issue when dealing with not a comparison and not  scalar
bitmask which fix is included here.

Still need to test it.

[Bug tree-optimization/110554] veclowering introduces invalid wide Boolean values

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110554

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #6)
> Created attachment 55472 [details]
> Fix veclowering types
> 
> So the problem is veclowering should have just use boolean_type_node here
> for the comparison.
> 
> There is another type issue when dealing with not a comparison and not 
> scalar bitmask which fix is included here.
> 
> Still need to test it.

I think this gets the case wrong where lowering splits a wide vector into
smaller vectors.  I think 'cond_type' is wrong in this case already,
but unconditionally using boolean_type_node looks wrong as well.
In the small vector case the code also uses COND_EXPR instead of
VEC_COND_EXPR.  Hmm, maybe we don't support big to small vector split
here at all ... in that case the patch looks good to me.

[Bug tree-optimization/110554] veclowering introduces invalid wide Boolean values

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110554

--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The trunk branch has been updated by Andrew Pinski <pinskia@gcc.gnu.org>:

https://gcc.gnu.org/g:a73b65b74105e76473cc2825bb4e7253deaf18b3

commit r14-2338-ga73b65b74105e76473cc2825bb4e7253deaf18b3
Author: Andrew Pinski <apinski@marvell.com>
Date:   Tue Jul 4 20:38:06 2023 -0700

    Fix PR 110554: vec lowering introduces scalar signed-boolean:32 comparisons

    So the problem is vector generic decided to do comparisons in
signed-boolean:32
    types but the rest of the middle-end was not ready for that. Since we are
building
    the comparison which will feed into a cond_expr here, using
boolean_type_node is
    better and also correct. The rest of the compiler thinks the ranges for
    comparison is always [0,1] too.

    Note this code does not currently lowers bigger vector sizes into smaller
    vector sizes so using boolean_type_node here is better.

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

    gcc/ChangeLog:

            PR middle-end/110554
            * tree-vect-generic.cc (expand_vector_condition): For comparisons,
            just build using boolean_type_node instead of the cond_type.
            For non-comparisons/non-scalar-bitmask, build a ` != 0` gimple
            that will feed into the COND_EXPR.

[Bug tree-optimization/110554] veclowering introduces invalid wide Boolean values

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110554

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |14.0
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #9 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Fixed.