<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">It is funny, both of those issues are things I actually asked people about, as I wasn’t sure what idioms were preferred. Patch committed with changes r209049.<div><br></div><div>Louis</div><div><br><div><div>On May 16, 2014, at 4:07 PM, Chandler Carruth <<a href="mailto:chandlerc@google.com">chandlerc@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div dir="ltr">Really cool.<div><br></div><div>Some comments on the patch itself:</div><div><br></div><div>+ unsigned DI = ~0x0U;</div><div><br></div><div>Maybe use an int and '-1'? Seems a bit simpler to read.</div>
<div><br></div><div><div>+ if (PtrOp->hasNUses(0)) {</div><div><br></div><div>->use_empty()</div><div><br></div><div>+ for (auto &I : PN->operands())</div><div>+ if (I->hasOneUse()) {</div><div>
+ // The one use is the node we are about to delete, so it is safe to</div><div>+ // this GEP.</div><div>+ dyn_cast<Instruction>(PtrOp)->eraseFromParent();</div><div>+ }</div><div>
+ dyn_cast<Instruction>(PtrOp)->eraseFromParent();</div><div>+ }</div></div><div><br></div><div>Why bother with this? I guess it doesn't hurt, but I thought instcombine would handle this kind of thing for you.</div>
<div><br></div><div>If you want to do it: RecursivelyDeleteTriviallyDeadInstructions</div><div><br></div><div>Feel free to submit whenever, these are all really minor comments.</div><div><br></div><div>PS: Any chance I can convince you to use phabricator? ;]</div>
</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, May 16, 2014 at 4:21 PM, Louis Gerbarg <span dir="ltr"><<a href="mailto:lgg@apple.com" target="_blank">lgg@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
The attached patch modifies InstCombine’s GEP code to merge GEPs that are split apart by PHIs. The existing code already generally merges GEP chains as aggressively as it can unless intermediary parts of the chain have additional uses that cannot be combined, so this patch makes the GEP chain folding more consistent overall. It also allows the target independent optimizer to hand more expressive GEPs to the backends, which can then use more complex addressing modes when appropriate. More details of the actual mechanics are in the patch.<br>
<br>
A concrete example of why want to do this is:<br>
<br>
unsigned testu(llvm::DenseMap<unsigned, unsigned> &dm, unsigned key) {<br>
return dm.lookup(key);<br>
}<br>
<br>
The generated code includes the following assembly:<br>
<br>
LBB0_6:<br>
leaq (%r8,%rdi,8), %rax<br>
movl 4(%rax), %eax<br>
<br>
which can be merged into a single instruction by the x86 backend provided the GEPs are merged across a PHI.<br>
<br>
LBB0_6: ## %if.then.i<br>
movl 4(%r8,%rdi,8), %eax<br>
<span class="HOEnZb"><font color="#888888"><br>
Louis<br>
<br>
<br>
</font></span><br>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div>
</blockquote></div><br></div></body></html>