On Tue, 2020-03-24 at 10:50 +0100, Jakub Jelinek wrote:
> On Mon, Mar 23, 2020 at 06:00:06PM -0600, Jeff Law via Gcc-patches wrote:
> > +/* Return true if CODE is valid for comparisons of mode MODE, false
> > +   otherwise.
> > +
> > +   It is always safe to return false, even if the code was valid for the
> > +   given mode as that will merely suppress optimizations.  */
> > +
> > +static bool
> > +comparison_code_valid_for_mode (enum rtx_code code, enum machine_mode mode)
> > +{
> > +  switch (code)
> > +    {
> > +      /* These are valid for integral, floating and vector modes.  */
> > +      case NE:
> > +      case EQ:
> > +      case GE:
> > +      case GT:
> > +      case LE:
> > +      case LT:
> > +   return (INTEGRAL_MODE_P (mode)
> > +           || FLOAT_MODE_P (mode)
> > +           || VECTOR_MODE_P (mode));
> 
> I'm not sure I understand the VECTOR_MODE_P cases.
> INTEGRAL_MODE_P or FLOAT_MODE_P already do include vector modes with the
> corresponding element types.
> And if you want the {,u}{fract,accum} modes for some of these, shouldn't
> they apply to both scalar and vector modes thereof?
> So, either drop the || VECTOR_MODE_P (mode), or use
> > > ALL_FRACT_MODE_P (mode) || ALL_ACCUM_MODE_P (mode)
> instead?
It's unclear to me as well.  I tried to follow rtl.def as closely as possible 
for
that reason.  rtl.def doesn't call out the fractional modes, accumulator modes,
etc.  It speaks strictly in terms of integral, float, vector mode classes.

+      /* These are filtered out in simplify_logical_operation, but
> > +    we check for them too as a matter of safety.   They are valid
> > +    for integral and vector modes.  */
> > +      case GEU:
> > +      case GTU:
> > +      case LEU:
> > +      case LTU:
> > +   return INTEGRAL_MODE_P (mode) || VECTOR_MODE_P (mode);
> 
> This doesn't look right.  I don't know what the fract/accum modes want, but
> you don't want to return true for GET_MODE_CLASS (mode) ==
> MODE_VECTOR_FLOAT, do you?
Again, this is a direct translation from the documentation in rtl.def.

I certainly don't mind changing it if there's a concrete proposal.

jeff

Reply via email to