This is an archive of the discontinued LLVM Phabricator instance.

AVX-512: Truncate with unsigned saturation.
ClosedPublic

Authored by delena on Jan 2 2017, 6:36 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

delena updated this revision to Diff 82802.Jan 2 2017, 6:36 AM
delena retitled this revision from to AVX-512: Truncate with unsigned saturation..
delena updated this object.
delena added reviewers: igorb, zvi, RKSimon.
delena set the repository for this revision to rL LLVM.
delena added a subscriber: llvm-commits.
zvi edited edge metadata.Jan 3 2017, 11:26 AM

Can you please add a test for VPMOVUSQW?

RKSimon edited edge metadata.Jan 3 2017, 11:43 AM

Any chance that we can use this to lower to PACKSS/PACKUS on older targets?

Any chance that we can use this to lower to PACKSS/PACKUS on older targets?

yes, of course. I can do this in one of the next patches.

delena updated this revision to Diff 83073.Jan 4 2017, 10:16 AM
delena edited edge metadata.

Added QW test as Zvi asked.

zvi added a comment.Jan 4 2017, 11:56 AM

Thanks, Elena. LGTM.

This revision was automatically updated to reflect the committed changes.
delena reopened this revision.Jan 10 2017, 4:54 AM
delena updated this revision to Diff 83798.Jan 10 2017, 4:59 AM

The previous patch was reverted due to a failure (https://llvm.org/bugs/show_bug.cgi?id=31589).

  1. I fixed the bug and added a lot of test cases.
  2. I promised to Simon to cover non-avx512 cases with VPACKUS. Now it is in. Additional tests are in avx-trunc.ll.
  1. I promised to Simon to cover non-avx512 cases with VPACKUS. Now it is in. Additional tests are in avx-trunc.ll.

Thank you!

../lib/Target/X86/X86ISelLowering.cpp
31130 ↗(On Diff #83798)

In.getScalarValueSizeInBits()

31144 ↗(On Diff #83798)

Can you use APIntOps::isMask here?

APIntOps::isMask(VT.getScalarSizeInBits(), C)
delena marked an inline comment as done.Jan 10 2017, 6:29 AM
delena added inline comments.
../lib/Target/X86/X86ISelLowering.cpp
31144 ↗(On Diff #83798)

Yes! I did not see this interface before. Thank you.

delena updated this revision to Diff 83804.Jan 10 2017, 6:31 AM

Some fixes after Simon's comments.

RKSimon accepted this revision.Jan 10 2017, 7:58 AM
RKSimon edited edge metadata.

LGTM with minors

../lib/Target/X86/X86ISelLowering.cpp
31137 ↗(On Diff #83804)

We canonicalize constants to RHS for UMIN/UMAX/SMIN/SMAX (see DAGCombiner::visitIMINMAX) so we only need this case.

../test/CodeGen/X86/avx512-trunc.ll
630 ↗(On Diff #83804)

Is it worth adding PR31589 as usat_trunc_db_1024_mem as well?

This revision is now accepted and ready to land.Jan 10 2017, 7:58 AM
This revision was automatically updated to reflect the committed changes.