<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Oct 23, 2014 at 11:51 PM, Robinson, Paul <span dir="ltr"><<a href="mailto:Paul_Robinson@playstation.sony.com" target="_blank">Paul_Robinson@playstation.sony.com</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">
<div lang="EN-US" link="blue" vlink="purple">
<div>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">See the existing commentary for TargetInfo::isCLZForZeroUndef. ARM, AArch64, Mips, PPC all have this as defined for zero input, see overrides in Targets.cpp.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">This is well-trod ground, and merely corrects a deficiency in the X86 info.</span></p></div></div></blockquote><div><br></div><div>I guess if there is precedent for this sort of thing then go ahead. For some reason I was interpreting the patch as adding the isCLZForZeroUndef interface.</div><div> </div><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"><div lang="EN-US" link="blue" vlink="purple"><div><p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"><u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">Andrea has a patch coming that does exactly the simplification you're suggesting, but driven by the flag passed in by Clang rather than a-priori knowledge about
instruction behavior.</span></p></div></div></blockquote><div><br></div><div>I don't see the point of requiring clang to inform the optimizer. No information is lost by the transformation.</div><div><br></div><div>-- Sean Silva</div><div> </div><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"><div lang="EN-US" link="blue" vlink="purple"><div><p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"><u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">I'm not willing to take on the chore of reassigning responsibility for which component knows exactly which arcana about particular instructions. I could see
adding a note to the description of the builtins.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">--paulr<u></u><u></u></span></p>
<p class="MsoNormal"><a name="14940ed480191ad5__MailEndCompose"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"><u></u> <u></u></span></a></p>
<div style="border-style:none none none solid;border-left-color:blue;border-left-width:1.5pt;padding:0in 0in 0in 4pt">
<div>
<div style="border-style:solid none none;border-top-color:rgb(181,196,223);border-top-width:1pt;padding:3pt 0in 0in">
<p class="MsoNormal"><b><span style="font-size:10pt;font-family:Tahoma,sans-serif">From:</span></b><span style="font-size:10pt;font-family:Tahoma,sans-serif"> Sean Silva [mailto:<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</a>]
<br>
<b>Sent:</b> Thursday, October 23, 2014 7:03 PM<br>
<b>To:</b> Robinson, Paul<br>
<b>Cc:</b> <a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<b>Subject:</b> Re: [PATCH][X86] __builtin_ctz/clz sometimed defined for zero input<u></u><u></u></span></p>
</div>
</div><div><div class="h5">
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">If I understand correctly, this patch is trying to change the meaning of __builtin_ctz (et al.) under some extremely specific conditions. I don't think that is the right direction since it will cause surprising undefined behavior bugs across
platforms. The intrinsic is documented to have undefined behavior in the 0 case (everywhere I looked, including our internal docs); a user that relies on the 0 case has a bug. It would be nice to add a UBSan check for this undefined behavior though to help
users fix their code.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">It would be better to just ensure that we always generate optimal code in the presence of a manual guard for the 0 case. For example, in the middle-end we could fold a manual 0 guard followed by @llvm.ctlz.*(X, true) into @llvm.ctlz.*(X,
false).<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">-- Sean Silva<u></u><u></u></p>
</div>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<p class="MsoNormal">On Thu, Oct 23, 2014 at 4:40 PM, Robinson, Paul <<a href="mailto:Paul_Robinson@playstation.sony.com" target="_blank">Paul_Robinson@playstation.sony.com</a>> wrote:<u></u><u></u></p>
<p class="MsoNormal" style="margin-bottom:12pt">In general, count-zeros instructions are undefined for a zero input value.<br>
However the X86 TZCNT and LZCNT instructions do return the bit-width on a<br>
zero input, so make Clang tell LLVM so.<br>
One quirk is that these instructions aren't necessarily both defined, so<br>
also create a separate predicate so we can do the right thing for all CPUs.<br>
--paulr<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><u></u><u></u></p>
</div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
</div></div></div>
</div>
</div>
</blockquote></div><br></div></div>