This is an archive of the discontinued LLVM Phabricator instance.

[TargetLowering] Add support for non-uniform vectors to BuildExactSDIV
ClosedPublic

Authored by RKSimon on Aug 7 2018, 9:12 AM.

Details

Summary

This patch refactors the existing BuildExactSDIV implementation to support non-uniform constant vector denominators.

I've ended up duplicating much of the scalar/vector code pattern that I did for TargetLowering::BuildUDIV (D49248) - calling a 'BuildSDIVPattern' helper - does anyone have any suggestions how I could reduce this further? I'm going to end up doing the same again for TargetLowering::BuildSDIV as well when I get around to it.

Diff Detail

Repository
rL LLVM

Event Timeline

RKSimon created this revision.Aug 7 2018, 9:12 AM
greened added a subscriber: greened.Aug 7 2018, 9:37 AM

Is it possible to break this into a NFC refactor and a separate patch to handle non-uniform denominators?

Is it possible to break this into a NFC refactor and a separate patch to handle non-uniform denominators?

Sure, pulling out the BuildSDIVPattern helper and the codegen is doable as an NFC. That would leave the patch mainly as the if(isVector()) .. else .. block

Sure, pulling out the BuildSDIVPattern helper and the codegen is doable as an NFC. That would leave the patch mainly as the if(isVector()) .. else .. block

The intent would be to have the largish code changes not result in test changes and then the smaller functional change would be reflected in the tests.

RKSimon updated this revision to Diff 159922.Aug 9 2018, 7:22 AM

Rebased.

I've refactored the approach slightly to demonstrate ISD::matchUnaryPredicate being used to iterate over a scalar or the elements of a vector. - this could be used in TargetLowering::BuildUDIV as well to make the equivalent code easier to understand.

craig.topper added inline comments.Aug 14 2018, 12:48 PM
test/CodeGen/X86/sdiv-exact.ll
2 ↗(On Diff #159922)

Why are the features flags between 32-bit and 64-bit mismatched? This results in making 64-bit look way better than 32-bit in the modified test cases. For example test5 where 64-bit is one instruction.

RKSimon added inline comments.Aug 14 2018, 1:05 PM
test/CodeGen/X86/sdiv-exact.ll
2 ↗(On Diff #159922)

Laziness mainly - I didn't see much gain from SSE2/AVX2 codegen tests for both 32/64 - its easy enough to add if you think its useful?

craig.topper accepted this revision.Aug 14 2018, 1:24 PM

LGTM

test/CodeGen/X86/sdiv-exact.ll
2 ↗(On Diff #159922)

I agree there's probably not much value. I just had to go up here to figure out why 64-bit looked so much better.

This revision is now accepted and ready to land.Aug 14 2018, 1:24 PM
This revision was automatically updated to reflect the committed changes.