<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Oct 27, 2014 at 5:20 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</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 dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5">On Sun, Oct 26, 2014 at 10:24 PM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.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 dir="ltr"><div class="gmail_extra">Sorry for the late replies, but as the last person to work on the CLZ / CTZ stuff, I have some small opinions here. =]</div><div class="gmail_extra"><br></div><div class="gmail_extra"><div class="gmail_quote"><span>On Sun, Oct 26, 2014 at 9:52 PM, David Majnemer <span dir="ltr"><<a href="mailto:david.majnemer@gmail.com" target="_blank">david.majnemer@gmail.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 dir="ltr">I think Paul's patch is fine.</div></blockquote><div><br></div></span><div>I firmly disagree.</div><span><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 dir="ltr"><div><br></div><div>Having `__builtin_ctz` asking the target if `isCLZForZeroUndef` seems busted and this patch seems to fix that just fine.<br></div><div></div></div></blockquote></span></div><br>Yes, that is busted and we should fix that aspect regardless. But changing the behavior on x86 seems actively harmful to portability.</div><div class="gmail_extra"><br></div><div class="gmail_extra"><br></div><div class="gmail_extra">So, let's look at the history.</div><div class="gmail_extra"><br></div><div class="gmail_extra">I went and taught LLVM to produce optimal code in cases where undef-at-zero was acceptable and only instructions with undef-at-zero were available. In order to generate reasonable code for these scenarios, Clang and LLVM had to respect the contract specified for the GCC __builtin_clz and __builtin_ctz. That spec[1] *very* clearly states that if the input is zero the result is undefined.</div><div class="gmail_extra"><br></div><div class="gmail_extra">1: <a href="https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html" target="_blank">https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html</a><br></div><div class="gmail_extra"><br></div><div class="gmail_extra">However, after this Bob changed ARM to not follow this spec citing a radar only in r149086. I objected at the time, and continue to feel this is the *wrong* approach for ARM. We should not be deviating from the clear, documented, and widely followed spec here. However, I had no real basis for arguing about the behavior of ARM at the time, and I doubt that I have much say in the behavior there even now. =] That said, on x86, I am firmly opposed to deviating from the clear and unambiguous spec.</div><div class="gmail_extra"><br></div><div class="gmail_extra">If folks want a builtin to access unambiguously defined-at-zero CLZ or CTZ behavior, we should define *new* builtins which have a clear spec here. However, I'm unpersuaded that this is better or more portable. The alternative code seems quite palatable: (x == 0 ? 32 : __builtin_clz(x))</div><div class="gmail_extra"><br></div><div class="gmail_extra">If LLVM fails to turn this into the maximally efficient code on some architecture, then much as others have suggested, we should enhance LLVM.</div></div></blockquote><div><br></div></div></div><div>I agree with Chandler on all points here.</div><div><br></div><div>__builtin_clz and __builtin_ctz is clearly and unambiguously defined by GCC's specification for them, and we should not be providing additional definedness  guarantees. If we want a defined-for-zero-input form of these intrinsics, they should have a different name (or an additional argument, or be written as "x == 0 ? 32 : __builtin_clz(x)" or similar).</div><div><br></div><div>It is a major feature of Clang that we try not to lock people in by providing incompatible extensions.</div></div></div></div></blockquote><div><br></div><div>Curiously, it looks like GCC is depending on definedness for 0 in its own headers: <a href="https://github.com/gcc-mirror/gcc/blob/master/gcc/config/i386/lzcntintrin.h">https://github.com/gcc-mirror/gcc/blob/master/gcc/config/i386/lzcntintrin.h</a></div><div><br></div><div>*shrug*</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">
<br>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">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><br>
<br></blockquote></div><br></div></div>