This is an archive of the discontinued LLVM Phabricator instance.

[X86] Add DAG combine to turn (trunc (srl (mul ext, ext), 16) into PMULHW/PMULHUW.
ClosedPublic

Authored by craig.topper on Apr 14 2018, 12:00 AM.

Details

Summary

Ultimately I want to use this to remove the intrinsics for these instructions.

Diff Detail

Event Timeline

craig.topper created this revision.Apr 14 2018, 12:00 AM

This doesn't seem x86 specific - any reason not to include it in dagcombine?

test/CodeGen/X86/pmulh.ll
7

Commit this with current codegen to show diff.

I thought about that. But it needs to be done pre-type legalization so that the extends haven’t been legalized yet. And it needs to be done on wider than legal vectors so the MULHU/MULHS get split during type legalization.

I think this means a generic DAG combine couldnt check isOperationLegal. Which sounds fine at first, but if the VT ends up needing to be promoted we’ll crash because type promotion doesn’t know how to promote MULHS/MULHU. So I wasn’t sure how to cleanly protect that from DAG combine.

spatel accepted this revision.Apr 21 2018, 7:18 AM

LGTM, but please do check in the tests with baseline output first (just in case the code change is reverted, we don't want to lose the tests).

This revision is now accepted and ready to land.Apr 21 2018, 7:18 AM
spatel added inline comments.Apr 21 2018, 7:23 AM
lib/Target/X86/X86ISelLowering.cpp
35997–35998

A top-level 'TODO' comment copying the reasoning for this being x86-specific would be good...in case someone fixes the common DAG problems and puts this in DAGCombiner, we'll know this can go.

36001

typo - 'instruction'

craig.topper closed this revision.Apr 21 2018, 12:03 PM

Committed in r330520. Forgot to add the Differential Revision line.