
<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><br></div>For the cases handled by this patch, the code produces CMOV ISD node, but even without this added section, the default is to produce a CMOV ISD node.<div>So in terms of whether to use CMOV for SELECT, I am following the default implementation.</div><div><br></div><div>This patch only handles sub+cmp for SELECT, a better implementation is to perform this optimization in peephole, then X86 will be handled in the same way as ARM.</div><div><br></div><div>It will be great if we can implement this in a target-independent way, but I don't think we can represent dependency on flags in a target-independent way?<br><div>If we remove 'cmp', we need to represent that 'sub' will update EFLAGS and there is a dependency from 'sub' to the SELECT due to EFLAGS.</div><div><br></div><div>Thanks,</div><div>Manman</div><div><br><div><div>On May 29, 2012, at 12:42 PM, Chandler Carruth wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div class="gmail_quote">On Tue, May 29, 2012 at 12:27 PM, Manman Ren <span dir="ltr"><<a href="mailto:mren@apple.com" target="_blank">mren@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<div style="word-wrap:break-word"><div><br></div><div><br></div>This patch is to replace sub+cmp with X86ISD::SUB when possible, it does not touch the decision of whether we should use 'cmp+jmp' or 'cmov'.</div>

</blockquote><div><br></div><div>Hmm, the oddity is that you do directly produce the CMOV ISD node. It looks like this will only fire on 'sub+cmp+cmov', not 'sub+cmp+jmp'. I suppose this is an artifact of this transform happening deep inside of the x86 backend, and so the decision of whether or not to use a branch vs. cmov has long since happened? Making sure I understand what you're doing here...</div>

<div><br></div><div>If I've understood this correctly, wouldn't that mean that the same logic will be needed again for code that emits the 'sub+cmp+jmp'? Is that already handled somewhere?</div><div><br></div>

<div>All in all, I'm agreeing with a comment on this bit of code: should this really be so specific? I'm seeing the possibility for a third copy of the core functionality here, it would be great if there were a way to put this into the dag-combine step, maybe even the target-independent dag-combine? Haven't thought much about the feasibility of that though...</div>

</div>

</blockquote></div><br></div></div></body></html>