This is an archive of the discontinued LLVM Phabricator instance.

[X86ISelLowering] Add additional support for multiplication-to-shift conversion.
ClosedPublic

Authored by chenli on Nov 11 2015, 8:56 PM.

Details

Summary

This patch adds support of conversion (mul x, 2^N + 1) => (add (shl x, N), x) and (mul x, 2^N - 1) => (sub (shl x, N), x) if the multiplication can not be converted to LEA + SHL or LEA + LEA. LLVM has already supported this on ARM, and it should also be useful on X86. Note the patch currently only applies to cases where the constant operand is positive, and I am planing to add another patch to support negative cases after this.

Diff Detail

Event Timeline

chenli updated this revision to Diff 40008.Nov 11 2015, 8:56 PM
chenli retitled this revision from to [X86ISelLowering] Add additional support for multiplication-to-shift conversion..
chenli updated this object.
chenli added a reviewer: craig.topper.
chenli added a subscriber: llvm-commits.
RKSimon added inline comments.Nov 28 2015, 6:41 AM
lib/Target/X86/X86ISelLowering.cpp
24313

There doesn't appear to be anything to prevent overflow here - e.g. for a 32-bit multiply by 0xFFFFFFFF. Do we need an initial patch that replaces the uint64_t MulAmt with APInt MulAmt?

chenli added inline comments.Nov 30 2015, 9:08 PM
lib/Target/X86/X86ISelLowering.cpp
24313

I think you are correct. I can create a separate patch to replace uint64_t with APInt in this function.

Add assertions to make sure overflow never happens.

RKSimon accepted this revision.Dec 11 2015, 1:09 AM
RKSimon edited edge metadata.

LGTM - with a couple of minor diffs.

lib/Target/X86/X86ISelLowering.cpp
24307

Rephrase the assert message to make it clear that they are the cases that would involve overflow.

test/CodeGen/X86/imul.ll
150

Please add overflow test cases - I don't see them in this file although they maybe somewhere else?

151

Missing newline

This revision is now accepted and ready to land.Dec 11 2015, 1:09 AM
chenli closed this revision.Dec 11 2015, 3:42 PM