Page MenuHomePhabricator

[X86] Improve mul w/ overflow codegen, to MUL8+SETO.

Authored by ab on Oct 15 2014, 3:39 PM.



Currently, @llvm.smul.with.overflow.i8 expands to 9 instructions, where
3 are really needed.

This adds X86ISD::UMUL8/SMUL8 SD nodes, and custom lowers them to

i8 is a special case because there is no two/three operand variants of
(I)MUL8, so the first operand and return value need to go in AL/AX.

Also, we can't write patterns for these instructions: TableGen refuses
patterns where output operands don't match SDNode results. In this case,
instructions where the output operand is an implicitly defined register.

A related special case (and FIXME) exists for MUL8 (

// FIXME: Used for 8-bit mul, ignore result upper 8 bits.
// This probably ought to be moved to a def : Pat<> if the
// syntax can be accepted.
[(set AL, (mul AL, GR8:$src)), (implicit EFLAGS)]

Ideally, these go away with UMUL8, but we still need to improve TableGen
support of implicit operands in patterns.

Before this change:

movsbl  %sil, %eax
movsbl  %dil, %ecx
imull   %eax, %ecx
movb    %cl, %al
sarb    $7, %al
movzbl  %al, %eax
movzbl  %ch, %esi
cmpl    %eax, %esi
setne   %al


movb    %dil, %al
imulb   %sil
seto    %al

Diff Detail


Event Timeline

ab updated this revision to Diff 14966.Oct 15 2014, 3:39 PM
ab retitled this revision from to [X86] Improve mul w/ overflow codegen, to MUL8+SETO..
ab updated this object.
ab edited the test plan for this revision. (Show Details)
ab set the repository for this revision to rL LLVM.
ab added a subscriber: Unknown Object (MLST).
ributzka accepted this revision.Oct 23 2014, 11:14 AM
ributzka added a reviewer: ributzka.
ributzka added a subscriber: ributzka.

Hi Ahmed,

this turned out to be a very nice and simple fix. I didn't know tblgen would give you such a hard time for this one.
Please check xaluo.ll for already existing tests for {u|s}mul.with.overflow and enable them for SelectionDAG.

Thanks and LGTM.


26–48 ↗(On Diff #14966)

There are already tests for this in xaluo.ll, but they are disabled for SelectionDAG.

This revision is now accepted and ready to land.Oct 23 2014, 11:14 AM
ab closed this revision.Oct 23 2014, 3:06 PM
ab updated this revision to Diff 15353.

Closed by commit rL220516 (authored by @ab).