<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Feb 23, 2015 at 2:17 PM, Hal Finkel <span dir="ltr"><<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class="">----- Original Message -----<br>
> From: "Francois Pichet" <<a href="mailto:pichet2000@gmail.com">pichet2000@gmail.com</a>><br>
> To: "LLVM Developers Mailing List" <<a href="mailto:llvmdev@cs.uiuc.edu">llvmdev@cs.uiuc.edu</a>><br>
> Sent: Sunday, February 22, 2015 5:34:11 PM<br>
> Subject: [LLVMdev] Question about shouldMergeGEPs in InstructionCombining<br>
><br>
> Hello<br>
><br>
> I am not sure I understand the logic for merging GEPs in<br>
> InstructionCombining.cpp:<br>
><br>
</span>> static bool shouldMergeGEPs (GEPOperator &GEP, GEPOperator &Src) {<br>
<span class="">> // If this GEP has only 0 indices, it is the same pointer as<br>
> // Src. If Src is not a trivial GEP too, don't combine<br>
> // the indices.<br>
> if (GEP.hasAllZeroIndices() && !Src.hasAllZeroIndices() &&<br>
> !Src.hasOneUse())<br>
> return false;<br>
> return true;<br>
> }<br>
><br>
><br>
><br>
> I have a case where<br>
> GEP: %arrayidx7 = getelementptr inbounds i32* %arrayidx, i32 %shl6<br>
> Src: %arrayidx = getelementptr inbounds [4096 x i32]* @phasor_4096,<br>
> i32 0, i32 %shl2<br>
><br>
><br>
</span>> GEP. hasAllZeroIndices() will return false and the merge will occur<br>
<span class="">> Why do we want to combine these 2 getelementptr?<br>
><br>
><br>
> On my out of tree target, combining these 2 GetElementPtr create a<br>
> performance regression because since GEP is in a loop (Src is out of<br>
> loop), GEP will lower to a more complicated address for a subsequent<br>
> load. (the complicated address needs to be calculated over and over<br>
> in the loop)<br>
><br>
<br>
</span>There are a couple of issues here. One, InstCombine's job is the move the IR toward a reasonable canonical form. It is doing that here, and I think that's the right thing to do. However, the problem you point out is a general one. Can you please explain why the MachineLICM pass is not able to hoist the redundant parts of the addressing computation out of the loop? We might also want to adjust for this in CodeGenPrep (CGP already has addressing-mode aware GEP splitting logic, although not quite for this case).<br>
<br></blockquote><div><br></div><div>Hi Hal,</div><div><br></div><div>MachineLICM is not able to hoist anything because the address mode is not loop invariant.</div><div><br></div><div>Here is a reduction of the code I am talking about.</div><div><br></div><div><div>extern const unsigned   phasor[4096];</div><div>void test(unsigned* out , unsigned step_size)</div><div>{</div><div><span style="white-space:pre">  </span>unsigned big_step_size = step_size<<2;</div><div><span style="white-space:pre">  </span>int *phasor_ptr_temp_1 = &phasor[big_step_size];</div><div>  for (int i = 0 ; i < 1020 ; i+=4)</div><div><span style="white-space:pre">    </span>out[i] = phasor_ptr_temp_1[i<<step_size];</div><div>}</div></div><div><br></div><div>I am getting slightly better code on my target (Octasic's Opus) if I return false for shouldMergeGEPs.</div><div>I just tried with ppc64 and x86-64 and I am also getting better code without the GEP merging in InstructionCombining. I am not sure what the solution is yet but I think we are too aggressive when merging GEPs in InstructionCombining.</div><div><br></div><div>Here is the details for ppc64: <a href="http://pastie.org/9978022">http://pastie.org/9978022</a></div><div><br></div><div><br></div><div><br></div><div> </div></div></div></div>