> > + if (reject_unsigned && TYPE_UNSIGNED (abs_type))
> > + return false;
> > + if (!ANY_INTEGRAL_TYPE_P (abs_type) || TYPE_OVERFLOW_WRAPS (abs_type))
> > + return false;
>
> Could you explain the reject_unsigned behaviour? I'd have expected
> TYPE_OVERFLOW_WRAPS (abs_type) to reject the unsigned case anyway.
When REJECT_UNSIGNED is true, cases wherein the abs type is unsigned or when the
unpromoted diff type is both not equal in precision to the abs type and unsigned
the statement is rejected.
vect_recog_absolute_difference replaces some of the logic in
vect_recog_sad_pattern
and is used by vect_recog_abd_pattern.
vect_recog_sad_pattern aborts if the abs type is unsigned or when the unprom
diff type isn't the same precision as abs type and unsigned.
vect_recog_abd_pattern doesn't do the same, so REJECT_UNSIGNED is a flag for
this.
I found it to be unnecessary as you suggested, so it's been dropped.
> > + if (half_type)
> > + {
> > + if (!SAME_TYPE (unprom[0].type, unprom[1].type))
> > + return NULL;
>
> I wouldn't have expected this to be unecessary. half_type is supposed
> to be a common type that can hold all values of unprom[0].type and
> unprom[1].type. We should just be able to do:
>
> > + tree diff_type = TREE_TYPE (diff_oprnds[0]);
> > + if (TYPE_PRECISION (out_type) != TYPE_PRECISION (diff_type))
> > + {
> > + tree vectype = get_vectype_for_scalar_type (vinfo, half_type);
> > + vect_convert_inputs (vinfo, stmt_vinfo, 2, abd_oprnds,
> > + half_type, unprom, vectype);
>
> ...this vect_convert_inputs unconditionally. We need to check that
> the get_vectype_for_scalar_type call succeeds though.
>
> So does it work as:
>
> if (half_type)
> {
> tree vectype = get_vectype_for_scalar_type (vinfo, half_type);
> if (!vectype)
> return false;
> vect_convert_inputs (vinfo, stmt_vinfo, 2, abd_oprnds,
> half_type, unprom, vectype);
> }
>
> ?
The proposed solution works.
> > + }
> > + else
> > + {
> > + abd_oprnds[0] = diff_oprnds[0];
> > + abd_oprnds[1] = diff_oprnds[1];
> > + }
> > + }
> > + else
> > + {
> > + if (unprom[0].op && unprom[1].op
> > + && (!SAME_TYPE (unprom[0].type, unprom[1].type)
> > + || !SAME_TYPE (unprom[0].type, out_type)))
> > + return NULL;
>
> AIUI, the !half_type case shouldn't look at unprom, since it's handling
> simple MINUS_EXPRs. I think we can just delete this "if" statement.
If statement removed.
> > + unprom[0].op = diff_oprnds[0];
> > + unprom[1].op = diff_oprnds[1];
> > + tree signed_out = signed_type_for (out_type);
> > + tree signed_out_vectype = get_vectype_for_scalar_type (vinfo,
> > signed_out);
>
> We need to check for success here too.
Add a check.
> > + vect_convert_inputs (vinfo, stmt_vinfo, 2, abd_oprnds,
> > + signed_out, unprom, signed_out_vectype);
> > +
> > + if (!SAME_TYPE (TREE_TYPE (diff_oprnds[0]), TREE_TYPE
> > (abd_oprnds[0])))
> > + return NULL;
>
> I don't think this is needed.
Statement removed.
> > + }
> > +
> > + if (!SAME_TYPE (TREE_TYPE (abd_oprnds[0]), TREE_TYPE (abd_oprnds[1]))
> > + || !SAME_TYPE (TREE_TYPE (abd_oprnds[0]), out_type))
> > + return NULL;
>
> I also don't think this is needed. AIUI, the previous code has done
> all the necessary correctness checks.
Statements removed.
> > + vect_pattern_detected ("vect_recog_abd_pattern", last_stmt);
> > +
> > + tree vectype = get_vectype_for_scalar_type (vinfo, out_type);
>
> I think instead we want the vector types computed above. That is:
>
> - The ABD should be done on the vector version of half_type
> if the subtraction was on promoted inputs. The result of
> the ABD should then be zero-extended (using vect_convert_output)
> to out_type.
>
> In particular, it's the sign of HALF_TYPE that decides whether
> it's signed or unsigned ABD.
>
> - The ABD should be done on the vector version of signed_outtype
> if the subtraction was on unpromoted inputs. We then might need
> to sign-cast it to outtype, if outtype is unsigned. We can
> use vect_convert_output for that too.
>
> In other words, this case must use signed ABD.
In the half_type case out_type is set to be half_type.
Patch is in the next response.
