This is an archive of the discontinued LLVM Phabricator instance.

[X86][SSE] Auto upgrade PADDS/PSUBS intrinsics to SADD_SAT/SSUB_SAT generic intrinsics (llvm)
ClosedPublic

Authored by RKSimon on Dec 19 2018, 11:17 AM.

Details

Summary

This auto upgrades the signed SSE saturated math intrinsics to SADD_SAT/SSUB_SAT generic intrinsics.

There are a few items to note:

msan_x86intrinsics.ll - I changed this to another similar x86 intrinsic instead of replacing it with the generic @llvm.sadd.sat.v8i16

x86-adds-subs.ll - Not sure what's best here, the diff is that the generic intrinsics don't constant fold undefs - do we want to add support for that? Should I move this out of the x86 folder and use generic intrinsics?

pic-load-remat.ll - This was using the old sse2 intrinsics, and now use the generics. I don't think it matters but I can change it to another x86 intrinsic if that helps. The test is very old, I'm not sure if its very useful tbh.

Clang counterpart: https://reviews.llvm.org/D55890

Diff Detail

Repository
rL LLVM

Event Timeline

RKSimon created this revision.Dec 19 2018, 11:17 AM
craig.topper added inline comments.Dec 19 2018, 11:33 AM
test/CodeGen/X86/pic-load-remat.ll
1 ↗(On Diff #178925)

Looks like its only looking for a load on psllw so it probably doesn't matter what else is going on. I'm fine with the generic intrinsics.

test/Transforms/InstCombine/X86/x86-adds-subs.ll
22 ↗(On Diff #178925)

Probably should move this out to generic. Feels weird to test upgraded intrinsics.

RKSimon marked an inline comment as done.Dec 19 2018, 12:57 PM
RKSimon added inline comments.
test/Transforms/InstCombine/X86/x86-adds-subs.ll
22 ↗(On Diff #178925)

OK - will do this as a pre-commit if that's OK? What about the UNDEF handling, just raise a bug?

craig.topper added inline comments.Dec 19 2018, 1:15 PM
test/Transforms/InstCombine/X86/x86-adds-subs.ll
22 ↗(On Diff #178925)

Thats fine with me. Raise a bug for undef handling. It look like for scalars we fold undef input to -1 if I'm reading the code in InstSimplify correctly. That looks to be different than what we were previously doing for the intrinsics.

RKSimon updated this revision to Diff 179058.EditedDec 20 2018, 6:04 AM

rebased

msan_x86intrinsics.ll - committed at rL349739

x86-adds-subs.ll - moved at rL349759 and raised undef regression at PR40110

pic-load-remat.ll - comitted at rL349742

This revision is now accepted and ready to land.Dec 20 2018, 1:02 PM
This revision was automatically updated to reflect the committed changes.