This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix bool vs. unsigned:1 vectorization (PR tree-optimization/79284)


On 02/01/2017 01:21 AM, Richard Biener wrote:

+/* Nonzero if TYPE represents a (scalar) boolean type or type
+   in the middle-end compatible with it.  */
+
+#define INTEGRAL_BOOLEAN_TYPE_P(TYPE) \
+  (TREE_CODE (TYPE) == BOOLEAN_TYPE            \
+   || ((TREE_CODE (TYPE) == INTEGER_TYPE       \
+       || TREE_CODE (TYPE) == ENUMERAL_TYPE)   \
+       && TYPE_PRECISION (TYPE) == 1           \
+       && TYPE_UNSIGNED (TYPE)))

(just to quote what you proposed).
And my suggestion is to move the PRECISION/UNSIGNED checks so that they're tested for BOOLEAN_TYPEs as well.



As of useless_type_conversion_p, I don't remember why we have

      /* Preserve conversions to/from BOOLEAN_TYPE if types are not
         of precision one.  */
      if (((TREE_CODE (inner_type) == BOOLEAN_TYPE)
           != (TREE_CODE (outer_type) == BOOLEAN_TYPE))
          && TYPE_PRECISION (outer_type) != 1)
        return false;

it came with r173854 where you see other BOOLEAN_TYPE
-> integral-type with precision 1 check changes, so a new predicate
is very welcome IMHO.
Funny, it's your change ;-) Looks like Kai added a gimple checker that verified that the operand of a TRUTH_NOT_EXPR must be compatible with boolean_type_node.

That check tripped (bz48989) on some Fortran code. Your fix seems to imply that the Fortran front-end created non-1-precision BOOLEAN_TYPE object.



all BOOLEAN_TYPEs but Adas have precision one and are unsigned
(their TYPE_SIZE may vary though).  Adas larger precision boolean
has only two valid values but needs to be able to encode some 'NaT'
state.

I think BOOLEAN_COMPATIBLE_TYPE_P would be misleading as it isn't
equal to types_compatible_p (boolean_type_node, t).

Maybe we want TWO_VALUED_UNSIGNED_INTEGRAL_TYPE_P ()? (ick)
I thought "BOOLEAN" covers TWO_VALUED_UNSIGNED well enough but
simply BOOLEAN_TYPE_P is easily confused with TREE_CODE () ==
BOOLEAN_TYPE.

I'm fine with changing the predicate to be more explicit, like

#define INTEGRAL_BOOLEAN_TYPE_P(TYPE) \
  (INTEGRAL_TYPE_P (TYPE) && TYPE_PRECISION (TYPE) == 1)

not sure if we really need the TYPE_UNSIGNED check?  The middle-end
has various places that just check for a 1-precision type when
asking for a boolean context.

So naming set aside, would you agree with the above definition?
(modulo a && TYPE_UNSIGNED (TYPE))?
I could agree to that. Alternately, we could restore the TYPE_PRECISION checks that Jakub removed in the vectorizer.

Jeff


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]