[llvm-dev] How to add optimizations to InstCombine correctly? (original) (raw)

Haidl, Michael via llvm-dev llvm-dev at lists.llvm.org
Tue Sep 19 04:23:26 PDT 2017

I am currently improving the D37896 to include the suggestions from Chad. However, running the lit checks for the x86 backend I observe some changes in the generated MC, e.g.:

llvm/test/CodeGen/X86/lea-3.ll:13:10: error: expected string not found in input ; CHECK: leal ([[A0]],[[A0]],2), %eax ^ :10:2: note: scanning from here orq %rdi, %rax ^ :10:2: note: with variable "A0" equal to "%rdi" orq %rdi, %rax ^ :10:2: note: with variable "A0" equal to "%rdi" orq %rdi, %rax ^ :23:2: note: possible intended match here leal (,%rdi,4), %eax ^ or:

llvm/test/CodeGen/X86/mul-constant-i16.ll:40:13: error: expected string not found in input ; X86-NEXT: movzwl {{[0-9]+}}(%esp), %eax ^ :35:2: note: scanning from here movzwl 4(%esp), %ecx ^ llvm/test/CodeGen/X86/mul-constant-i16.ll:272:13: error: expected string not found in input ; X86-NEXT: movzwl {{[0-9]+}}(%esp), %eax ^ :212:2: note: scanning from here movzwl 4(%esp), %ecx ^

What is the right way to fix this? Is it ok to modify the tests to match the new generated pattern?

Cheers, Michael

Am 16.09.2017 um 15:46 schrieb Simon Pilgrim:

This conversation has (partially) moved on to D37896 now, but if possible I was hoping that we could perform this in DAGCombiner and remove the various target specific combines that we still have.

At least ARM/AARCH64 and X86 have cases that can hopefully be generalised and removed, but there will probably be a few legality/perf issues that will occur. Simon.

On 14 Sep 2017, at 06:23, Craig Topper <craig.topper at gmail.com_ _<mailto:craig.topper at gmail.com>> wrote:

Probably in visitMUL in DAGCombiner.cpp to be target independent. Or in LowerMUL in X86ISelLowering.cpp to be X86 specific. Adding Simon. Simon, which were you thinking? ~Craig On Wed, Sep 13, 2017 at 10:06 PM, Haidl, Michael <michael.haidl at uni-muenster.de <mailto:michael.haidl at uni-muenster.de>> wrote: Hi Craig, thanks for digging into this. So InstCombine is the wrong place for fixing PR34474. Can you give me a hint where such an optimization should go into CodeGen? I am not really familiar with stuff that happens after the MidLevel. Cheers, Michael Am 13.09.2017 um 19:21 schrieb Craig Topper: > And that is less instructions. So from InstCombine's perspective the > multiply is the correct answer. I think this transformation is better > left to codegen where we know whether multiply or shift is truly better. > > ~Craig > > On Wed, Sep 13, 2017 at 10:18 AM, Craig Topper <craig.topper at gmail.com <mailto:craig.topper at gmail.com> > <mailto:craig.topper at gmail.com <mailto:craig.topper at gmail.com>>> wrote: > > There is in fact a transform out there somewhere that reverses yours. > > define i64 @foo(i64 %a) { > %b = shl i64 %a, 5 > %c = add i64 %b, %a > ret i64 %c > } > > becomes > > define i64 @foo(i64 %a) { > > %c = mul i64 %a, 33 > > ret i64 %c > > } > > > ~Craig > > On Wed, Sep 13, 2017 at 10:11 AM, Craig Topper > <craig.topper at gmail.com <mailto:craig.topper at gmail.com> <mailto:craig.topper at gmail.com <mailto:craig.topper at gmail.com>>> wrote: > > Your code seems fine. InstCombine can infinite loop if some > other transform is reversing your transform. Can you send the > whole patch and a test case? > > ~Craig > > On Wed, Sep 13, 2017 at 10:01 AM, Haidl, Michael via llvm-dev > <llvm-dev at lists.llvm.org_ _<mailto:llvm-dev at lists.llvm.org> <mailto:llvm-dev at lists.llvm.org_ _<mailto:llvm-dev at lists.llvm.org>>> wrote: > > Hi, > > I am working on PR34474 and try to add a new optimization to > InstCombine. Like in other parts of the visitMul function I > add a Shl > through the IR builder and create a new BinaryOp which I > return from > visitMul. If I understand correctly the new BinaryOp > returned from > visitMul should replace the original Instruction in the > Worklist. > However, I end up in an infinite loop and the Instruction I > try to > replace gets scheduled again and again. What is wrong in my > code? > > // Replace X * (2^C+/-1) with (X << C) -/+ X_ _> APInt Plus1 = *IVal + 1; > APInt Minus1 = *IVal - 1; > int isPow2 = Plus1.isPowerOf2() ? 1 : Minus1.isPowerOf2() ? > -1 : 0; > > if (isPow2) { > APInt &Pow2 = isPow2 > 0 ? Plus1 : Minus1; > Value *Shl = Builder.CreateShl(Op0, Pow2.logBase2()); > return BinaryOperator::Create(isPow2 > 0 ? > BinaryOperator::Sub : > BinaryOperator::Add, Shl, Op0); > } > > Thanks, > Michael

More information about the llvm-dev mailing list