<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Oct 24, 2014 at 5:15 AM, Andrea Di Biagio <span dir="ltr"><<a href="mailto:andrea.dibiagio@gmail.com" target="_blank">andrea.dibiagio@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Sean,<br>
<span class=""><br>
On Fri, Oct 24, 2014 at 3:02 AM, Sean Silva <<a href="mailto:chisophugis@gmail.com">chisophugis@gmail.com</a>> wrote:<br>
><br>
> If I understand correctly, this patch is trying to change the meaning of<br>
> __builtin_ctz (et al.) under some extremely specific conditions. I don't<br>
> think that is the right direction since it will cause surprising undefined<br>
> behavior bugs across platforms.<br>
<br>
</span>Paul's fix would only affect x86 behavior.<br>
What it does is simply that on some x86 cpus, ctz/clz is defined on<br>
zero (i.e. we have an instruction for it).<br>
I don't think it can cause surprising undefined behavior across platforms.<br>
<span class=""><br>
> The intrinsic is documented to have<br>
> undefined behavior in the 0 case (everywhere I looked, including our<br>
> internal docs); a user that relies on the 0 case has a bug. It would be nice<br>
> to add a UBSan check for this undefined behavior though to help users fix<br>
> their code.<br>
<br>
</span>The intrinsic is documented to have the following behavior<br>
(<a href="http://llvm.org/docs/LangRef.html#llvm-cttz-intrinsic" target="_blank">http://llvm.org/docs/LangRef.html#llvm-cttz-intrinsic</a>):<br>
"If src == 0 then the result is the size in bits of the type of src if<br>
is_zero_undef == 0 and undef otherwise."<br>
Where 'is_zero_undef' is the second argument to the intrinsic.<br>
So, the intrinsic is not documented to have always undefined behavior<br>
in the zero case.<br>
Also I don't think that relying on the 0 case is a bug. It is a very<br>
reasonable assumption on all modern x86 architectures.<br></blockquote><div><br></div><div>If the compiler's documentation says "X is undefined", it is a bug to rely on X. Period. There is a contract between the compiler and the user which is essential for the compiler to remain maintainable. The contract is the stated semantics of source code constructs (described in language standards and/or the compiler's documentation). You are conflating changing the contract with violating the contract. It is definitely a bug if a user's source code depends on behavior that violates the contract. It is a completely separate discussion as to whether or not a particular change is a reasonable modification of the contract that the compiler developers and users can agree on.</div><div><br></div><div>-- Sean Silva</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
> It would be better to just ensure that we always generate optimal code in<br>
> the presence of a manual guard for the 0 case. For example, in the<br>
> middle-end we could fold a manual 0 guard followed by @llvm.ctlz.*(X, true)<br>
> into @llvm.ctlz.*(X, false).<br>
<br>
</span>I agree that the codegen should be improved in that case.<br>
<br>
My concern is that your suggested approach would force people to<br>
always guard calls to __builtin_ctz/__builtin_clz against zero.<br>
>From a customer point of view, the compiler knows exactly if ctz and<br>
clz is defined on zero. It is basically pushing the problem on the<br>
customer by forcing them to guard all the calls to ctz/clz against<br>
zero. We've already had a number of customer queries/complaints about<br>
this and I personally don't think it is unreasonable to have ctz/clz<br>
defined on zero on our target (and other x86 targets where the<br>
behavior on zero is clearly defined).<br>
<span class="HOEnZb"><font color="#888888"><br>
-Andrea<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
><br>
> -- Sean Silva<br>
><br>
> On Thu, Oct 23, 2014 at 4:40 PM, Robinson, Paul<br>
> <<a href="mailto:Paul_Robinson@playstation.sony.com">Paul_Robinson@playstation.sony.com</a>> wrote:<br>
>><br>
>> 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<br>
>> CPUs.<br>
>> --paulr<br>
>><br>
>><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>
><br>
><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>
</div></div></blockquote></div><br></div></div>