<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">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.<div class=""><br class=""></div><div class="">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.</div><div class=""><br class=""></div><div class="">Simon.<br class=""><div class=""> <br class=""><div><blockquote type="cite" class=""><div class="">On 14 Sep 2017, at 06:23, Craig Topper <<a href="mailto:craig.topper@gmail.com" class="">craig.topper@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">Probably in visitMUL in DAGCombiner.cpp to be target independent. Or in LowerMUL in X86ISelLowering.cpp to be X86 specific.<div class=""><br class=""></div><div class="">Adding Simon. Simon, which were you thinking?</div></div><div class="gmail_extra"><br clear="all" class=""><div class=""><div class="gmail_signature" data-smartmail="gmail_signature">~Craig</div></div>
<br class=""><div class="gmail_quote">On Wed, Sep 13, 2017 at 10:06 PM, Haidl, Michael <span dir="ltr" class=""><<a href="mailto:michael.haidl@uni-muenster.de" target="_blank" class="">michael.haidl@uni-muenster.de</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Craig,<br class="">
<br class="">
thanks for digging into this. So InstCombine is the wrong place for<br class="">
fixing PR34474. Can you give me a hint where such an optimization should<br class="">
go into CodeGen? I am not really familiar with stuff that happens after<br class="">
the MidLevel.<br class="">
<br class="">
Cheers,<br class="">
Michael<br class="">
<span class=""><br class="">
Am 13.09.2017 um 19:21 schrieb Craig Topper:<br class="">
> And that is less instructions. So from InstCombine's perspective the<br class="">
> multiply is the correct answer. I think this transformation is better<br class="">
> left to codegen where we know whether multiply or shift is truly better.<br class="">
><br class="">
> ~Craig<br class="">
><br class="">
> On Wed, Sep 13, 2017 at 10:18 AM, Craig Topper <<a href="mailto:craig.topper@gmail.com" class="">craig.topper@gmail.com</a><br class="">
</span><span class="">> <mailto:<a href="mailto:craig.topper@gmail.com" class="">craig.topper@gmail.com</a><wbr class="">>> wrote:<br class="">
><br class="">
> There is in fact a transform out there somewhere that reverses yours.<br class="">
><br class="">
> define i64 @foo(i64 %a) {<br class="">
> %b = shl i64 %a, 5<br class="">
> %c = add i64 %b, %a<br class="">
> ret i64 %c<br class="">
> }<br class="">
><br class="">
> becomes<br class="">
><br class="">
> define i64 @foo(i64 %a) {<br class="">
><br class="">
> %c = mul i64 %a, 33<br class="">
><br class="">
> ret i64 %c<br class="">
><br class="">
> }<br class="">
><br class="">
><br class="">
> ~Craig<br class="">
><br class="">
> On Wed, Sep 13, 2017 at 10:11 AM, Craig Topper<br class="">
</span><span class="">> <<a href="mailto:craig.topper@gmail.com" class="">craig.topper@gmail.com</a> <mailto:<a href="mailto:craig.topper@gmail.com" class="">craig.topper@gmail.com</a><wbr class="">>> wrote:<br class="">
><br class="">
> Your code seems fine. InstCombine can infinite loop if some<br class="">
> other transform is reversing your transform. Can you send the<br class="">
> whole patch and a test case?<br class="">
><br class="">
> ~Craig<br class="">
><br class="">
> On Wed, Sep 13, 2017 at 10:01 AM, Haidl, Michael via llvm-dev<br class="">
</span><div class=""><div class="h5">> <<a href="mailto:llvm-dev@lists.llvm.org" class="">llvm-dev@lists.llvm.org</a> <mailto:<a href="mailto:llvm-dev@lists.llvm.org" class="">llvm-dev@lists.llvm.<wbr class="">org</a>>> wrote:<br class="">
><br class="">
> Hi,<br class="">
><br class="">
> I am working on PR34474 and try to add a new optimization to<br class="">
> InstCombine. Like in other parts of the visitMul function I<br class="">
> add a Shl<br class="">
> through the IR builder and create a new BinaryOp which I<br class="">
> return from<br class="">
> visitMul. If I understand correctly the new BinaryOp<br class="">
> returned from<br class="">
> visitMul should replace the original Instruction in the<br class="">
> Worklist.<br class="">
> However, I end up in an infinite loop and the Instruction I<br class="">
> try to<br class="">
> replace gets scheduled again and again. What is wrong in my<br class="">
> code?<br class="">
><br class="">
> // Replace X * (2^C+/-1) with (X << C) -/+ X<br class="">
> APInt Plus1 = *IVal + 1;<br class="">
> APInt Minus1 = *IVal - 1;<br class="">
> int isPow2 = Plus1.isPowerOf2() ? 1 : Minus1.isPowerOf2() ?<br class="">
> -1 : 0;<br class="">
><br class="">
> if (isPow2) {<br class="">
> APInt &Pow2 = isPow2 > 0 ? Plus1 : Minus1;<br class="">
> Value *Shl = Builder.CreateShl(Op0, Pow2.logBase2());<br class="">
> return BinaryOperator::Create(isPow2 > 0 ?<br class="">
> BinaryOperator::Sub :<br class="">
> BinaryOperator::Add, Shl, Op0);<br class="">
> }<br class="">
><br class="">
> Thanks,<br class="">
> Michael</div></div></blockquote></div></div>
</div></blockquote></div><br class=""></div></div></body></html>