This is an archive of the discontinued LLVM Phabricator instance.

[X86] Replace support for vXi32 SMUL_LOHI/UMUL_LOHI with MULHS/MULHU support instead.
ClosedPublic

Authored by craig.topper on Aug 24 2018, 11:41 PM.

Details

Summary

The only time vector SMUL_LOHI/UMUL_LOHI nodes are created is during division/remainder lowering. If its created before op legalization, generic DAGCombine immediately turns that SMUL_LOHI/UMUL_LOHI into a MULHS/MULHU since only the upper half is used. That node will stick around through vector op legalization and will be turned back into UMUL_LOHI/SMUL_LOHI during op legalization. It will then be custom lowered by the X86 backend. Due to this two step lowering the vector shuffles created by the custom lowering get legalized after their inputs rather than before. This prevents the shuffles from being combined with any build_vector of constants.

This patch uses changes vXi32 to use MULHS/MULHU instead. This is what the later DAG combine did anyway. But by skipping the change back to UMUL_LOHI/SMUL_LOHI we lower it before any constant BUILD_VECTORS. This allows the vector_shuffle creation to constant fold with the build_vectors. This accounts for the test changes here.

Diff Detail

Event Timeline

craig.topper created this revision.Aug 24 2018, 11:41 PM
RKSimon accepted this revision.Aug 25 2018, 3:19 AM

LGTM with a few minors - cheers!

lib/Target/X86/X86ISelLowering.cpp
22899

Worth pulling out Op0/Op1/NumElts/IsSigned out into common code? We're repeating it in iXi8 lowering as well

22943

Remove braces

This revision is now accepted and ready to land.Aug 25 2018, 3:19 AM
craig.topper added inline comments.Aug 25 2018, 10:56 AM
lib/Target/X86/X86ISelLowering.cpp
22899

I'll do it as a follow up since everything is very slightly different. Op0/Op1 in i32 and A/B in i8. NumElts is int in i32 and unsigned in i8. i8 is checking for !IsSigned really.

This revision was automatically updated to reflect the committed changes.