[cfe-commits] r149086 - in /cfe/trunk: include/clang/Basic/TargetInfo.h lib/Basic/Targets.cpp lib/CodeGen/CGBuiltin.cpp test/CodeGen/builtin-count-zeros.c
bob.wilson at apple.com
Thu Jan 26 15:02:05 PST 2012
On Jan 26, 2012, at 2:38 PM, Chandler Carruth wrote:
> On Thu, Jan 26, 2012 at 2:14 PM, Bob Wilson <bob.wilson at apple.com> wrote:
> Make clz/ctz builtins defined for zero on ARM targets. rdar://10732455
> ARM supports clz and ctz directly and both operations have well-defined
> results for zero. There is no disadvantage in performance to using the
> defined-at-zero versions of llvm.ctlz/cttz intrinsics. We're running into
> ARM-specific code written with the assumption that __builtin_clz(0) == 32,
> even though that value is technically undefined. The code is failing now
> because of llvm optimizations that are taking advantage of the undef
> behavior (specifically svn r147255). There's nothing wrong with that
> optimization on x86 where any incorrect assumptions about __builtin_clz(0)
> will quickly be exposed. For ARM, though, optimizations based on that undef
> behavior are likely to cause subtle bugs. Other targets with defined-at-zero
> clz/ctz support may want to override the default behavior as well.
> Ouch. I can't argue at all with the logic here, but this is a nasty, nasty minefield.
> I wonder, would it be better to add new builtins to represent defined-at-zero semantics consistently on all platforms? Maybe we could get folks to switch to those (or at least start).
Adding new built-ins would be a good idea, but I think this behavior should stay regardless. There is already code out there for ARM with bad assumptions, and if anyone wants to write code that is portable across other compilers for ARM, they will have to stick with the existing built-ins.
> Either way, please document this language extension really loudly, as it is a specific divergence from GCC's specification of the builtins, and the builtins are supposedly Clang providing a "faithful" emulation of GCC….
How is it a divergence? GCC says that the values are undefined. This does not conflict with that. It makes no difference on ARM codegen so the only real effect is to disable optimizations that try to take advantage of that undef.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the cfe-commits