This is an archive of the discontinued LLVM Phabricator instance.

[ARM][VecReduce] Force expand vector_reduce_fmin
ClosedPublic

Authored by dmgreen on Feb 3 2020, 10:21 AM.

Details

Summary

Under MVE, we do not have any lowering for fminimum, which a vector_reduce_fmin without NoNan will be expanded into. As with the other recent patches, force this to expand in the pre-isel pass. Note that Neon lowering would be OK because the scalar fminimum uses the vector VMIN instruction, but is probably better to just rely on the scalar operations, which is what is done here.

Also fixes what appears to be the reversal of INF vs -INF in the vector_reduce_fmin widening code.

Diff Detail

Event Timeline

dmgreen created this revision.Feb 3 2020, 10:21 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 3 2020, 10:21 AM
nikic accepted this revision.Feb 3 2020, 11:41 AM

LGTM. Thanks for catching the issue in widening as well.

As a side note, the current fmin/fmax expansion in ExpandReductions is not well-defined (it uses an ordered cmp + select expansion on the assumption that only fast fmin/fmax reductions may exist). But that's a separate and preexisting issue.

This revision is now accepted and ready to land.Feb 3 2020, 11:41 AM

Thanks. I left a fixme in the tests about the fmin/max expansion. Hopefully it makes it obvious that it's not unexpected that these tests change.