On Tue, Jan 31, 2017 at 04:31:28PM -0700, Jeff Law wrote:
> On 01/31/2017 04:22 PM, Jakub Jelinek wrote:
> > On Tue, Jan 31, 2017 at 03:52:10PM -0700, Jeff Law wrote:
> > > Which makes your patch safe -- but introduces a non-obvious dependency
> > > between useless_type_conversion_p and your definition of
> > > INTEGRAL_BOOLEAN_TYPE and how it's used in the vectorizer.
> >
> > The predicate is simply true for all BOOLEAN_TYPEs and all types that are
> > compatible with it in the middle-end. BOOLEAN_TYPEs with different
> > precisions are not considered compatible types, therefore they won't appear
> > together without explicit casts in between.
> I understand that. My objection is that there's a highly non-obvious
> dependency between useless_type_conversion, INTEGRAL_TYPE_P (in particular
> how its used in the tree-vect-patterns).
>
> If someone came along and changed useless_type_conversion, they'd have no
> way to know they need to go fix up INTEGRAL_TYPE_P and/or the vectorizer at
> the same time.
I think we have many other places that depend on such behavior of u_t_c,
after all, the patch decreases the amount of such spots by replacing 3
places doing such checks manually with the macro that does the check.
> Thus my suggestion that you explicitly check the precision and rename the
> macro. THough I don't offhand have a better suggestion.
Not sure I understand what you mean explicitly check the precision,
the macro checks the precision already, and intentionally only for
non-BOOLEAN_TYPE. If you mean checking precision explicitly in the spots
where the macro is used, that would be worse for the hypothetical case when
u_t_c would change in this regard, you'd have far more places to change.
As for macro name, I came up e.g. with BOOLEAN_COMPATIBLE_P or
BOOLEAN_COMPATIBLE_TYPE_P, perhaps those could make it clearer on what it
is.
Jakub
