<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="">A matter of perspective. The patch results in better code for asm intrinsics that allow one input parameter in a register or memory. Do you agree?<div class=""><br class=""></div><div class="">I understand the desire for generalization, but there needs to be better reasoning about its benefits. We don’t have many test cases with intrinsics that specify rm inputs, and I don’t think I have seen one with two rm parameters yet. </div><div class=""><br class=""></div><div class="">-Gerolf</div><div class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Apr 27, 2015, at 9:55 PM, Chandler Carruth <<a href="mailto:chandlerc@google.com" class="">chandlerc@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">But this isn't a correctness issue that merits an expedient fix even if we can't fix this the right way, and this isn't a useful incremental step code wise towards handling these things in the fundamentally right way.<br class=""><div class=""><br class=""></div><div class="">I'm not sure why we would need to have cg copy prop handle things... Or do you just mean the general idea of handling inline asm operands in the register allocator as vregs? While I genuinely think we should do that at some point, I feel like we could do a much more principled approach without going there just through local analysis:</div><div class=""><br class=""></div><div class="">1) Count the operands that require registers first (potentially per register class)</div><div class="">2) Pick registers for the remaining "rm" operands where we have available registers in the right class</div><div class="">3) Leave the remaining "rm" operands as memory</div><div class=""><br class=""></div><div class="">What I'd really like to see is for us to actually analyze the inline asm (do we do that yet?) and count the number of uses of each operand so we can both a) make the most frequently used "rm" operands be the ones in registers and b) use especially densely encoded registers like A on x86 for the most commonly used operands (potentially when doing so doesn't introduce a copy).</div><div class=""><br class=""></div><div class="">Perhaps you have even better ideas of where this part of LLVM should go?</div><div class=""><br class=""></div><div class="">Overall, I would be more comfortable of this if it were clear that it's actually moving LLVM toward a more generally powerful handling rather than just fixing an extremely narrow and arbitrary special case with special case logic that won't ever really generalize.</div></div><br class=""><div class="gmail_quote">On Mon, Apr 27, 2015 at 8:09 PM Gerolf Hoflehner <<a href="mailto:ghoflehner@apple.com" class="">ghoflehner@apple.com</a>> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Eric<br class="">
<br class="">
yes, there are more general ways of handling this, and for a single operand your suggestion improves on the implementation I currently have. Wrt to the more general solution it seems that it could result in a problem - at least when implemented naively like for one operand at a time - e.g. when an operand resides in a location needed by another one. This is indicated by a comment in ChooseConstraint() " Ideally, we would pick the most specific constraint possible: if we have something that fits into a register, we would pick it. The problem here is that if we have something that could either be in a register or in memory that use of the register could cause selection of *other* operands to fail: they might only succeed if we pick memory. ".<br class="">
The most general route for handling this would be to enhance cg copy prop to handle cases like vreg->mem etc. But that is more longer term and would be an overkill for the isolated instance at hand.<br class="">
So I think I go ahead and add a location check to my current code. Would that stand a chance to pass your LGTM threshold?<br class="">
<br class="">
Cheers<br class="">
Gerolf<br class="">
<br class="">
<br class="">
<a href="http://reviews.llvm.org/D9271" target="_blank" class="">http://reviews.llvm.org/D9271</a><br class="">
<br class="">
EMAIL PREFERENCES<br class="">
<a href="http://reviews.llvm.org/settings/panel/emailpreferences/" target="_blank" class="">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br class="">
<br class="">
<br class="">
<br class="">
_______________________________________________<br class="">
llvm-commits mailing list<br class="">
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank" class="">llvm-commits@cs.uiuc.edu</a><br class="">
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank" class="">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br class="">
</blockquote></div>
</div></blockquote></div><br class=""></div></body></html>