[PATCH][X86] __builtin_ctz/clz sometimed defined for zero input

[Sean Silva]

On Fri, Oct 24, 2014 at 8:23 AM, Robinson, Paul <
Paul_Robinson at playstation.sony.com> wrote:

> 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.


What is the harm of documenting it? By defining this behavior we are
pledging to forever support it. Users should feel free to use this feature,
and we should not be afraid to claim we support it. E.g. an addendum "when
targeting BMI-capable x86 chips that have the LZCNT instruction, this
intrinsic produces the same result in the 0 case as the LZCNT instruction".
It will be a nightmare for users if it "seems to work, but from the
documentation it looks like the compiler isn't guaranteed to do this".

-- Sean Silva



>   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:andrea.dibiagio at gmail.com]
> > Sent: Friday, October 24, 2014 5:15 AM
> > To: Sean Silva
> > Cc: Robinson, Paul; cfe-commits at cs.uiuc.edu
> > 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 <chisophugis at gmail.com>
> 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
> > > <Paul_Robinson at playstation.sony.com> 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
> > >> cfe-commits at cs.uiuc.edu
> > >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> > >>
> > >
> > >
> > > _______________________________________________
> > > cfe-commits mailing list
> > > cfe-commits at cs.uiuc.edu
> > > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> > >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141024/5dd171b5/attachment.html>