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

Robinson, Paul Paul_Robinson at playstation.sony.com
Fri Oct 24 08:23:07 PDT 2014


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




More information about the cfe-commits mailing list