Hi Andrea,
You are correct about the behavior of the LLVM intrinsic, however Sean is
concerned about the behavior of the Clang builtin, which is imitating the
GNU builtin, which says "If x is 0, the result is undefined.".
https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html
What I did was change the definition of the undefined behavior of the Clang
builtin, which is defined to be anything we want, including something sensible.
On reflection, I think it would NOT be appropriate to document this predictable
behavior of the Clang builtins, because people who are writing source-code
calls
should follow the documented behavior and treat them as having undefined
behavior
for 0 input. However we are free to *implement* the builtin however we like
for
0 input, which we have already done for various other architectures, and we are
now doing the same for X86.
This allows us to optimize
unsigned foo(unsigned x) { return (x ? __builtin_clz(x) : 32) }
into a single LZCNT instruction on a BMI-capable X86, if we choose.
But anybody who writes
unsigned foo(unsigned x) { return __builtin_clz(x); }
is living dangerously and we should not imply otherwise.
--paulr
> -----Original Message-----
> From: Andrea Di Biagio [mailto:[email protected]]
> Sent: Friday, October 24, 2014 5:15 AM
> To: Sean Silva
> Cc: Robinson, Paul; [email protected]
> Subject: Re: [PATCH][X86] __builtin_ctz/clz sometimed defined for zero
> input
>
> 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.
>
> > 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).
>
> I agree that the codegen should be improved in that case.
>
> My concern is that your suggested approach would force people to
> always guard calls to __builtin_ctz/__builtin_clz against zero.
> From a customer point of view, the compiler knows exactly if ctz and
> clz is defined on zero. It is basically pushing the problem on the
> customer by forcing them to guard all the calls to ctz/clz against
> zero. We've already had a number of customer queries/complaints about
> this and I personally don't think it is unreasonable to have ctz/clz
> defined on zero on our target (and other x86 targets where the
> behavior on zero is clearly defined).
>
> -Andrea
>
> >
> > -- Sean Silva
> >
> > On Thu, Oct 23, 2014 at 4:40 PM, Robinson, Paul
> > <[email protected]> wrote:
> >>
> >> In general, count-zeros instructions are undefined for a zero input
> value.
> >> However the X86 TZCNT and LZCNT instructions do return the bit-width on
> a
> >> zero input, so make Clang tell LLVM so.
> >> One quirk is that these instructions aren't necessarily both defined,
> so
> >> also create a separate predicate so we can do the right thing for all
> >> CPUs.
> >> --paulr
> >>
> >>
> >> _______________________________________________
> >> cfe-commits mailing list
> >> [email protected]
> >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> >>
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > [email protected]
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> >
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
