This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Add tests showing missed vector saturating add/sub combines
ClosedPublic

Authored by frasercrmck on Jul 23 2021, 4:26 AM.

Details

Summary

These will be optimized by upcoming patches. The tests are primarily not
being optimized due to the lack of support for saturating vector
arithmetic in the RISC-V backend.

On top of that, however, a large percentage of the scalable-vector tests
are also lacking support in the DAGCombiner: either in
ISD::matchBinaryPredicate or due to checks specifically for
BUILD_VECTOR and not SPLAT_VECTOR.

Diff Detail

Event Timeline

frasercrmck created this revision.Jul 23 2021, 4:26 AM
frasercrmck requested review of this revision.Jul 23 2021, 4:26 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 23 2021, 4:26 AM
craig.topper accepted this revision.Jul 26 2021, 11:59 AM

LGTM

llvm/test/CodeGen/RISCV/rvv/combine-sats.ll
84

I assume this gets expanded because the legalizer is looking for UMIN/UMAX to be Legal rather than Custom?

This revision is now accepted and ready to land.Jul 26 2021, 11:59 AM
frasercrmck marked an inline comment as done.Jul 27 2021, 1:18 AM
frasercrmck added inline comments.
llvm/test/CodeGen/RISCV/rvv/combine-sats.ll
84

Yeah, pretty much. I think this is a pretty weird path to take, at least for our target. The sub (umin) is being unconditionally combined to usubsat (pre legalization) and then the usubsat is being legalized. The legalizer checks whether umin is legal, which isn't not for fixed vectors, so it's expanded to usubo (and some other ops) which is in turn legalized to a sub and setcc.

I wonder if this should be fixed. Checking "legal or custom" in the "addsubsat" expansion seems like the right fix in principle. It was actually "legal or custom" until fairly recently -- D91876 -- when some X86 combine was made generic. I don't understand the X86 target well enough to know if this is a significant obstacle to overcome.

This revision was landed with ongoing or failed builds.Jul 27 2021, 1:20 AM
This revision was automatically updated to reflect the committed changes.
frasercrmck marked an inline comment as done.