[PATCH] D24956: [SelectionDAG] Add expansion and promotion of [US]MUL_LOHI

[Eli Friedman via Phabricator via llvm-commits]

efriedma accepted this revision.
efriedma added a comment.
This revision is now accepted and ready to land.

LGTM with minor comments addressed.



================
Comment at: lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:2210
     // Hacker's Delight (itself derived from Knuth's Algorithm M from section
     // 4.3.1).
     unsigned Bits = NVT.getSizeInBits();
----------------
nhaehnle wrote:
> efriedma wrote:
> > Can you replace this code with a call to expandMUL?
> I played around with that, but I'd rather not do it in this patch. The generated code sequences are different, and in particular we'd generate non-legal instructions that at least the X86 target doesn't expect and doesn't handle properly. This is visible in test/CodeGen/X86/imul-256/512/1024.ll.
Okay, that's fine.


================
Comment at: lib/CodeGen/SelectionDAG/TargetLowering.cpp:3155
+  if (!VT.isVector() && DAG.MaskedValueIsZero(LHS, HighMask) &&
+      DAG.MaskedValueIsZero(RHS, HighMask)) {
     // The inputs are both zero-extended.
----------------
MaskedValueIsZero should do the right thing for vectors, I think, with recent changes to computeKnownBits.


================
Comment at: lib/CodeGen/SelectionDAG/TargetLowering.cpp:3181
+  unsigned ShiftAmount = OuterBitSize - InnerBitSize;
+  EVT ShiftAmountTy = getShiftAmountTy(VT, DAG.getDataLayout());
+  if (APInt::getMaxValue(ShiftAmountTy.getSizeInBits()).ult(ShiftAmount)) {
----------------
Hmm...

I was going to say that ShiftAmount always fits into ShiftAmountTy because it's a defined shift (ShiftAmount < OuterBitSize < maximum unsigned value), but getShiftAmountTy doesn't always return a sensible result for illegal types.  Please just note this problem with a FIXME.


https://reviews.llvm.org/D24956