On Fri, Oct 24, 2014 at 11:45 PM, Sean Silva <[email protected]> wrote: > > > On Fri, Oct 24, 2014 at 5:15 AM, Andrea Di Biagio > <[email protected]> wrote: >> >> Hi Sean, >> >> On Fri, Oct 24, 2014 at 3:02 AM, Sean Silva <[email protected]> wrote: >> > >> > 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. >> >> Paul's fix would only affect x86 behavior. >> What it does is simply that on some x86 cpus, ctz/clz is defined on >> zero (i.e. we have an instruction for it). >> I don't think it can cause surprising undefined behavior 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. >> >> The intrinsic is documented to have the following behavior >> (http://llvm.org/docs/LangRef.html#llvm-cttz-intrinsic): >> "If src == 0 then the result is the size in bits of the type of src if >> is_zero_undef == 0 and undef otherwise." >> Where 'is_zero_undef' is the second argument to the intrinsic. >> So, the intrinsic is not documented to have always undefined behavior >> in the zero case. >> Also I don't think that relying on the 0 case is a bug. It is a very >> reasonable assumption on all modern x86 architectures. > > > 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. >
My bad. I was only looking at the semantic of the llvm cttz/ctlz intrinsics. As you said, __builtin_clz/ctz is always undefined on zero. Relying on that undefined is clearly wrong. Sorry for the confusion. I definitely agree with you that we should try to generate optimal code in the presence of a guard for the 0 case. A guard for 0 followed by a call to the intrinsic ctz/clz with "undefined at zero" semantic could be simplified to a call to intrinsic ctz/clz that sets the "defined at zero" flag. _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
