<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><meta http-equiv="Content-Type" content="text/html charset=utf-8" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">Hi all,<div class=""><br class=""></div><div class="">As a continuation of this thread, I was about to fill a bug report requesting the modification of <font size="3" class=""><span style="white-space: pre-wrap;" class="">DAGTypeLegalizer::ExpandIntRes_SIGN_EXTEND, in order to avoid the creation of Shifts for targets with no native support. For example by generating a ‘select' equivalent to a<0 ? -1 : 0 </span></font>instead of an arithmetic shift right. For targets with no multiple shifts or word extension instructions, the select is much cheaper.</div><div class=""><br class=""></div><div class="">However, I found that the early InstCombine pass spoils such optimisation by creating shifts on their own as a transform of (supposedly) equivalent code. </div><div class=""><br class=""></div><div class="">Consider this:</div><div class=""><br class=""></div><div class=""><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco; background-color: rgb(255, 255, 255);" class=""><span style="color: #ba2da2" class="">int</span> word_extend( <span style="color: #ba2da2" class="">int</span> a )</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco; background-color: rgb(255, 255, 255);" class="">{</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco; background-color: rgb(255, 255, 255);" class=""> <span style="color: #ba2da2" class="">return</span> a<<span style="color: #272ad8" class="">0</span> ? -<span style="color: #272ad8" class="">1</span> : <span style="color: #272ad8" class="">0</span>;</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco; background-color: rgb(255, 255, 255);" class="">}</div></div><div class=""><br class=""></div><div class="">This is converted into this IR-Code (It’s 16 bits because it’s the MSP430 target)</div><div class=""><br class=""></div><div class=""><div class=""><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco; background-color: rgb(255, 255, 255);" class="">; Function Attrs: norecurse nounwind readnone</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco; background-color: rgb(255, 255, 255);" class="">define dso_local i16 @word_extend(i16 %a) local_unnamed_addr #0 {</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco; background-color: rgb(255, 255, 255);" class="">entry:</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco; background-color: rgb(255, 255, 255);" class=""> %a.lobit = ashr i16 %a, 15</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco; background-color: rgb(255, 255, 255);" class=""> ret i16 %a.lobit</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco; background-color: rgb(255, 255, 255);" class="">}</div></div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco; background-color: rgb(255, 255, 255);" class=""><br class=""></div><div style="margin: 0px; line-height: normal; background-color: rgb(255, 255, 255);" class="">Which the backend then turns into this</div><div style="margin: 0px; line-height: normal; background-color: rgb(255, 255, 255);" class=""><br class=""></div><div style="margin: 0px; line-height: normal; background-color: rgb(255, 255, 255);" class=""><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco;" class=""><span class="Apple-tab-span" style="white-space:pre"> </span>.globl<span class="Apple-tab-span" style="white-space:pre"> </span>word_extend</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco;" class=""><span class="Apple-tab-span" style="white-space:pre"> </span>.p2align<span class="Apple-tab-span" style="white-space:pre"> </span><span style="color: #272ad8" class="">1</span></div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco;" class=""><span class="Apple-tab-span" style="white-space:pre"> </span>.type<span class="Apple-tab-span" style="white-space:pre"> </span>word_extend,@function</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco;" class="">word_extend:</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco;" class=""><span class="Apple-tab-span" style="white-space:pre"> </span>swpb<span class="Apple-tab-span" style="white-space:pre"> </span>r12</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco;" class=""><span class="Apple-tab-span" style="white-space:pre"> </span>sxt<span class="Apple-tab-span" style="white-space:pre"> </span>r12</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco;" class=""><span class="Apple-tab-span" style="white-space:pre"> </span>rra<span class="Apple-tab-span" style="white-space:pre"> </span>r12</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco;" class=""><span class="Apple-tab-span" style="white-space:pre"> </span>rra<span class="Apple-tab-span" style="white-space:pre"> </span>r12</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco;" class=""><span class="Apple-tab-span" style="white-space:pre"> </span>rra<span class="Apple-tab-span" style="white-space:pre"> </span>r12</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco;" class=""><span class="Apple-tab-span" style="white-space:pre"> </span>rra<span class="Apple-tab-span" style="white-space:pre"> </span>r12</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco;" class=""><span class="Apple-tab-span" style="white-space:pre"> </span>rra<span class="Apple-tab-span" style="white-space:pre"> </span>r12</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco;" class=""><span class="Apple-tab-span" style="white-space:pre"> </span>rra<span class="Apple-tab-span" style="white-space:pre"> </span>r12</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco;" class=""><span class="Apple-tab-span" style="white-space:pre"> </span>rra<span class="Apple-tab-span" style="white-space:pre"> </span>r12</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco;" class=""><span class="Apple-tab-span" style="white-space:pre"> </span>ret</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco;" class="">.Lfunc_end2:</div></div><div style="margin: 0px; line-height: normal; background-color: rgb(255, 255, 255);" class=""><br class=""></div><div style="margin: 0px; line-height: normal; background-color: rgb(255, 255, 255);" class="">For this particular target, a simple select would be much cheaper. Ideally, the frontend should have generated this:</div><div style="margin: 0px; line-height: normal; background-color: rgb(255, 255, 255);" class=""><br class=""></div><div style="margin: 0px; line-height: normal; background-color: rgb(255, 255, 255);" class=""><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco;" class="">; Function Attrs: norecurse nounwind readnone</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco;" class="">define dso_local i16 @word_extend(i16 %a) local_unnamed_addr #0 {</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco;" class="">entry:</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco;" class=""> %cmp = icmp slt i16 %a, 0 </div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco;" class=""> %cond = select i1 %cmp, i16 -1, i16 0</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco;" class=""> ret i16 %cond</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco;" class="">}</div></div><div class=""><br class=""></div><div class=""><span style="background-color: rgb(255, 255, 255);" class="">Which the backend would convert into basically a ‘move', and a ‘branch' skipping an instruction, which would be much cheaper.</span></div><div class=""><span style="background-color: rgb(255, 255, 255);" class=""><br class=""></span></div><div class=""><span style="background-color: rgb(255, 255, 255);" class="">Unfortunately, the above IR code is combined back into a SignExtend by the backend, thus recreating the undesired shift, and producing again bad code for the MSP430.</span></div><div class=""><span style="background-color: rgb(255, 255, 255);" class=""><br class=""></span></div><div class=""><span style="background-color: rgb(255, 255, 255);" class="">The following IR code also causes sub-optimal code on the MSP430 due to combines into ZeroExtend and then into shifts in </span><span style="font-family: Monaco; font-size: 11px; background-color: rgb(255, 255, 255); color: rgb(79, 129, 135);" class="">DAGCombiner</span><span style="font-family: Monaco; font-size: 11px; background-color: rgb(255, 255, 255);" class="">::SimplifySelectC,</span><span style="font-family: Monaco; font-size: 11px; background-color: rgb(255, 255, 255);" class=""> </span><span style="font-family: Monaco; font-size: 11px; background-color: rgb(255, 255, 255); color: rgb(79, 129, 135);" class="">DAGCombiner</span><span style="font-family: Monaco; font-size: 11px; background-color: rgb(255, 255, 255);" class="">::foldSelectCCToShiftAnd </span><span style="background-color: rgb(255, 255, 255);" class="">and </span><span style="font-family: Monaco; font-size: 11px; background-color: rgb(255, 255, 255); color: rgb(186, 45, 162);" class="">static</span><span style="font-family: Monaco; font-size: 11px; background-color: rgb(255, 255, 255);" class=""> </span><span style="font-family: Monaco; font-size: 11px; background-color: rgb(255, 255, 255); color: rgb(79, 129, 135);" class="">SDValue</span><span style="font-family: Monaco; font-size: 11px; background-color: rgb(255, 255, 255);" class=""> foldExtendedSignBitTest :</span></div><div class=""><span style="background-color: rgb(255, 255, 255);" class=""><br class=""></span></div><div class=""><div style="background-color: rgb(255, 255, 255); margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco;" class="">; Function Attrs: norecurse nounwind readnone</div><div style="background-color: rgb(255, 255, 255); margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco;" class="">define dso_local i16 @word_extend(i16 %a) local_unnamed_addr #0 {</div><div style="background-color: rgb(255, 255, 255); margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco;" class="">entry:</div><div style="background-color: rgb(255, 255, 255); margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco;" class=""> %cmp = icmp slt i16 %a, 0 </div><div style="background-color: rgb(255, 255, 255); margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco;" class=""> %cond = select i1 %cmp, i16 1, i16 0</div><div style="background-color: rgb(255, 255, 255); margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco;" class=""> ret i16 %cond</div><div style="background-color: rgb(255, 255, 255); margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco;" class="">}</div></div><div style="background-color: rgb(255, 255, 255); margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco;" class=""><br class=""></div><div style="background-color: rgb(255, 255, 255); margin: 0px; line-height: normal;" class="">Variations like this also cause the same issue:</div><div style="background-color: rgb(255, 255, 255); margin: 0px; line-height: normal;" class=""><br class=""></div><div style="background-color: rgb(255, 255, 255); margin: 0px; line-height: normal;" class=""><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco;" class="">; Function Attrs: norecurse nounwind readnone</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco;" class="">define dso_local i16 @word_extend(i16 %a) local_unnamed_addr #0 {</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco;" class="">entry:</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco;" class=""> %cmp = icmp slt i16 %a, 0 </div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco;" class=""> %cond = select i1 %cmp, i16 2, i16 0</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco;" class=""> ret i16 %cond</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco;" class="">}</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco;" class=""><br class=""></div></div><div class=""><span style="background-color: rgb(255, 255, 255);" class="">So for now, we would need to fix these:</span></div><div class=""><span style="background-color: rgb(255, 255, 255);" class=""><br class=""></span></div><div class=""><span style="font-family: Monaco; font-size: 11px; background-color: rgb(255, 255, 255); color: rgb(79, 129, 135);" class="">DAGCombiner</span><span style="font-family: Monaco; font-size: 11px; background-color: rgb(255, 255, 255);" class="">::SimplifySelectCC</span></div><div class=""><span style="font-family: Monaco; font-size: 11px; background-color: rgb(255, 255, 255); color: rgb(79, 129, 135);" class="">DAGCombiner</span><span style="font-family: Monaco; font-size: 11px; background-color: rgb(255, 255, 255);" class="">::foldSelectCCToShiftAnd</span></div><div class=""><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco; background-color: rgb(255, 255, 255);" class=""><span style="color: #ba2da2" class="">static</span> <span style="color: #4f8187" class="">SDValue</span> foldExtendedSignBitTest</div></div><div class=""><span style="background-color: rgb(255, 255, 255);" class=""><br class=""></span></div><div class=""><span style="background-color: rgb(255, 255, 255);" class="">I think this can be fixed in a similar way than </span><a href="https://reviews.llvm.org/D68397" class="">https://reviews.llvm.org/D68397</a></div><div class=""><span style="background-color: rgb(255, 255, 255);" class="">I posted a bug report for that <a href="https://bugs.llvm.org/show_bug.cgi?id=43559" class="">https://bugs.llvm.org/show_bug.cgi?id=43559</a></span></div><div class=""><br class=""></div><div class=""><span style="background-color: rgb(255, 255, 255);" class="">I strongly suggest the above gets fixed. HOWEVER, even after the DAG combiner is fixed, the issues will remain due to InstCombine doing essentially the same thing </span><span style="background-color: rgb(255, 255, 255);" class="">much earlier. Consider code like this:</span></div><div class=""><span style="background-color: rgb(255, 255, 255);" class=""><br class=""></span></div><div class=""><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco; background-color: rgb(255, 255, 255);" class=""><span style="color: rgb(186, 45, 162);" class="">int</span> test0( <span style="color: rgb(186, 45, 162);" class="">int</span> a )</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco; background-color: rgb(255, 255, 255);" class="">{</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco; background-color: rgb(255, 255, 255);" class=""> <span style="color: rgb(186, 45, 162);" class="">return</span> a<<span style="color: rgb(39, 42, 216);" class="">0</span> ? -<span style="color: rgb(39, 42, 216);" class="">1</span> : <span style="color: rgb(39, 42, 216);" class="">0</span>;</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco; background-color: rgb(255, 255, 255);" class="">}</div></div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco; background-color: rgb(255, 255, 255);" class=""><br class=""></div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Monaco; background-color: rgb(255, 255, 255);" class=""><div style="margin: 0px; line-height: normal;" class=""><span style="color: rgb(186, 45, 162);" class="">int</span> test1( <span style="color: rgb(186, 45, 162);" class="">int</span> a )</div><div style="margin: 0px; line-height: normal;" class="">{</div><div style="margin: 0px; line-height: normal;" class=""> <span style="color: rgb(186, 45, 162);" class="">return</span> a<<span style="color: rgb(39, 42, 216);" class="">0</span> ? <span style="color: rgb(39, 42, 216);" class="">1</span> : <span style="color: rgb(39, 42, 216);" class="">0</span>;</div><div style="margin: 0px; line-height: normal;" class="">}</div><div style="margin: 0px; line-height: normal;" class=""><br class=""></div><div style="margin: 0px; line-height: normal;" class=""><div style="margin: 0px; line-height: normal;" class=""><span style="color: rgb(186, 45, 162);" class="">int</span> test2( <span style="color: rgb(186, 45, 162);" class="">int</span> a )</div><div style="margin: 0px; line-height: normal;" class="">{</div><div style="margin: 0px; line-height: normal;" class=""> <span style="color: rgb(186, 45, 162);" class="">return</span> a<<span style="color: rgb(39, 42, 216);" class="">0</span> ? <font color="#272ad8" class="">2</font> : <span style="color: rgb(39, 42, 216);" class="">0</span>;</div><div style="margin: 0px; line-height: normal;" class="">}</div><div style="margin: 0px; line-height: normal;" class=""><br class=""></div><div style="margin: 0px; line-height: normal;" class=""><div style="margin: 0px; line-height: normal;" class=""><span style="color: rgb(186, 45, 162);" class="">int</span> test3( <span style="color: rgb(186, 45, 162);" class="">int</span> a )</div><div style="margin: 0px; line-height: normal;" class="">{</div><div style="margin: 0px; line-height: normal;" class=""> <span style="color: rgb(186, 45, 162);" class="">return</span> a<<span style="color: rgb(39, 42, 216);" class="">0</span> ? <font color="#272ad8" class="">3</font> : <span style="color: rgb(39, 42, 216);" class="">0</span>;</div><div style="margin: 0px; line-height: normal;" class="">}</div></div></div></div><div class=""><span style="background-color: rgb(255, 255, 255);" class=""><br class=""></span></div><div class=""><span style="background-color: rgb(255, 255, 255);" class="">In all cases, InstCombine converts the above into Shifts, right into the IR, which the backend can’t do anything about. This is why I suggest that something must be done at the IR Optimize level to deal with such undesired situations. I think it should not be that difficult for someone with experience on it, because incidentally, the -O0 option almost produces the desired result, except for all that stack storage of variables and registers.</span></div><div class=""><span style="background-color: rgb(255, 255, 255);" class=""><br class=""></span></div><div class=""><span style="background-color: rgb(255, 255, 255);" class="">I hope that I was able to explain the nature of the problem in an effective way.</span></div><div class=""><span style="background-color: rgb(255, 255, 255);" class=""><br class=""></span></div><div class=""><span style="background-color: rgb(255, 255, 255);" class="">Thanks.</span></div><div class=""><span style="background-color: rgb(255, 255, 255);" class=""><br class=""></span></div><div class=""><span style="background-color: rgb(255, 255, 255);" class="">John</span></div><div class=""><span style="background-color: rgb(255, 255, 255);" class=""><br class=""></span></div><div class=""><br class=""><div class=""><blockquote type="cite" class=""><div class="">On 3 Oct 2019, at 11:26, Joan Lluch <<a href="mailto:joan.lluch@icloud.com" class="">joan.lluch@icloud.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><meta http-equiv="Content-Type" content="text/html charset=utf-8" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">Hi all,<div class=""><br class=""><div class=""><blockquote type="cite" class=""><div class="">On 2 Oct 2019, at 14:34, Sanjay Patel <<a href="mailto:spatel@rotateright.com" class="">spatel@rotateright.com</a>> wrote</div><div class=""><div dir="ltr" class=""><div dir="ltr" class=""><div class="">Providing target options/overrides to code that is supposed to be target-independent sounds self-defeating to me. I doubt that proposal would gain much support.</div><div class="">Of course, if you're customizing LLVM for your own out-of-trunk backend, you can do anything you'd like if you're willing to maintain the custom patches.<br class=""></div><div class="">I suggest that you file a bug report showing an exact problem caused by 1 of the icmp/select transforms. <br class=""></div><div class="">If it can be done using an in-trunk backend like AVR or MSP430, then we'll have a real example showing the harm and hopefully better ideas about how to solve the bug.</div></div></div></div></blockquote></div><div class=""><div class=""><br class=""></div><div class="">I fully understand your statement about target-independent code. This has no possible discussion in a general way. However one of my points is that at least some of this code is not really that “target independent” because in fact it depends on cheap features available on ALL targets, which is not the case. That’s why I think that some flexibility must be given. I’m not advocating for “target options/overrides”, but general ones that can be optionally chosen by particular targets. I think that at the end of the day it’s just a matter of language, not a matter of fact.</div><div class=""><br class=""></div><div class="">If we look at the problem from some distance, we find that there are already options to alter IR code generation. For example the -phi-node-folding-threshold option is one of them. Ok, no target seems to use that particular one, but it’s there already, and it modifies LLVM-IR generation by telling how aggressively LLVM turns branches into selects. I found that this option produces better LLVM-IR input for the target I’m working on, as well as both the MSP430 and AVR targets. </div><div class=""><br class=""></div><div class="">I can of course try to apply my custom patches and maintain them, but although I was able to implement a backend, the complexity and extend of the whole thing is beyond my abilities and resources. Furthermore, I regard this as a LLVM wide problem, and that’s why I try to raise some awareness.</div><div class=""><br class=""></div><div class="">Following your suggestion, I just filed a bug with a very simple example that to make things easier is not attributable to the InstCombine pass in this case but to transformations happening during LLVM codegen. </div><div class=""><a href="https://bugs.llvm.org/show_bug.cgi?id=43542" class="">https://bugs.llvm.org/show_bug.cgi?id=43542</a></div><div class=""><br class=""></div><div class="">John</div><div class=""><br class=""><div class=""><blockquote type="cite" class=""><div class="">On 2 Oct 2019, at 14:34, Sanjay Patel <<a href="mailto:spatel@rotateright.com" class="">spatel@rotateright.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><div dir="ltr" class=""><div dir="ltr" class="gmail_attr">On Tue, Oct 1, 2019 at 5:51 PM Joan Lluch <<a href="mailto:joan.lluch@icloud.com" class="">joan.lluch@icloud.com</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div style="overflow-wrap: break-word;" class="">In
order to give better support to current and future implementations of
small processor targets, I wonder if instead of attempting to implement a
general solution, we could implement a set of compiler flags (or code
hooks) that targets could use to optionally disable undesired IR
optimiser actions, however not affecting anything of the current
behaviour as long as these flags are not used. If we had that, nothing
would need to be done on the existing targets, particularly the major
ones, and yet, new backend developments and small processor (AVR,
MSP430) could potentially benefit from that. My opinion is that the
availability of such options will not defeat the “almost entirely
target-independent” nature of the IR optimiser, as the transformations
would still be target-agnostic, except that targets would be able to
decide which ones are more convenient for them, or disable the non
desirable ones. Does this sound ok or feasible?.</div></blockquote><div class=""><br class=""></div><div class="">Providing target options/overrides to code that is supposed to be target-independent sounds self-defeating to me. I doubt that proposal would gain much support.</div><div class="">Of course, if you're customizing LLVM for your own out-of-trunk backend, you can do anything you'd like if you're willing to maintain the custom patches.<br class=""></div><div class="">I suggest that you file a bug report showing an exact problem caused by 1 of the icmp/select transforms. <br class=""></div><div class="">If it can be done using an in-trunk backend like AVR or MSP430, then we'll have a real example showing the harm and hopefully better ideas about how to solve the bug.<br class=""></div><div class=""> </div></div><br class=""><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Oct 1, 2019 at 5:51 PM Joan Lluch <<a href="mailto:joan.lluch@icloud.com" class="">joan.lluch@icloud.com</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div style="overflow-wrap: break-word;" class=""><div class="">Hi Sanjay,</div><div class=""><br class=""></div><div class="">Thanks for your reply.</div><div class=""><br class=""></div><div class=""><blockquote type="cite" class=""><div dir="ltr" class=""><div class="">So yes, the IR optimizer (instcombine is the specific pass) sometimes turns icmp (and select) sequences into ALU ops. Instcombine is almost entirely *target-independent* and should remain that way. The (sometimes unfortunate) decision to create shifts were made based on popular targets of the time (PowerPC and/or x86), and other targets may have suffered because of that.</div></div></blockquote><br class=""></div><div class="">Yes, that’s what I actually found that I didn’t anticipate.</div><div class=""><br class=""></div><div class=""><blockquote type="cite" class=""><div dir="ltr" class=""><div class="">We've been trying to reverse those canonicalizations in IR over the past few years when the choice is clearly not always optimal, but it's not easy. To avoid perf regressions, you first have to make the backend/codegen aware of the alternate pattern that includes icmp/select and transform that to math/logic (translate instcombine code to DAGCombiner). Then, you have to remove the transform from instcombine and replace it with the reverse transform. This can uncover infinite loops and other problems within instcombine.</div></div></blockquote><br class=""></div><div class="">I understand that. In any case, I am glad that at least this is acknowledged as some kind of flaw of the LLVM system, particularly for the optimal implementation of small processor backends. As these targets generally have cheap branches and do not generally have selects or multi-bit shifts, they hardly benefit from transformations involving shifts or aggressively attempting to replace jumps.</div><div class=""><br class=""></div><div class=""><blockquote type="cite" class=""><div dir="ltr" class=""><div class="">So to finally answer the question: If you can transform the shift into an alternate sequence with a "setcc" DAG node in your target's "ISelLowering" code, that's the easiest way forward. Otherwise, you have to weigh the impact of each target-independent transform on every target.</div></div></blockquote><br class=""></div><div class="">Yes, that’s what I have been doing all the time. My backend contains a lot of code only to reverse undesired LLVM transformations, and yet the resulting final assembly is not as good as it could be, because it is often hard or impossible to identify all sources of improvement. It’s ironical that some older, less sophisticated compilers (and GCC) produce /much/ better code than LLVM for simple architectures.</div><div class=""><br class=""></div><div class="">In order to give better support to current and future implementations of small processor targets, I wonder if instead of attempting to implement a general solution, we could implement a set of compiler flags (or code hooks) that targets could use to optionally disable undesired IR optimiser actions, however not affecting anything of the current behaviour as long as these flags are not used. If we had that, nothing would need to be done on the existing targets, particularly the major ones, and yet, new backend developments and small processor (AVR, MSP430) could potentially benefit from that. My opinion is that the availability of such options will not defeat the “almost entirely target-independent” nature of the IR optimiser, as the transformations would still be target-agnostic, except that targets would be able to decide which ones are more convenient for them, or disable the non desirable ones. Does this sound ok or feasible?.</div><div class=""><br class=""></div><div class="">John</div><div class=""><br class=""></div></div></blockquote></div></div>
</div></blockquote></div><br class=""></div></div></div></div></div></blockquote></div><br class=""></div></div></div></body></html>