Is it OK to merge the following revision(s) to the 6.0 branch?
https://reviews.llvm.org/rL324593
Let's put this on hold until https://reviews.llvm.org/D43146 has been landed.
Can you provide more context on what this is fixing? It needs to get committed soon if it's to make the 6.0 release.
__builtin_clz is int -> int, and often implemented using a native clz (or similar) instruction. However, on 64-bit architecture which don't have such an instruction, rather than calling the __clzsi2 function (as one would expect, given SI is the GCC internal name for int), it instead uses __clzdi2, the 64-bit (DI) version. Since compiler-rt's implementation of __clzdi2 uses a divide-and-conquer approach of calling __builtin_clz on each half of the 64-bit value, on these architectures, the __builtin_clz ends up calling __clzdi2 itself, thus leading to non-terminating recursion, whereas the intention is that, if there's no clz instruction, __clzdi2 ends up calling compiler-rt's __clzsi2. Thus the patch simply identifies these problematic platforms and re-defines __builtin_clz to be __clzsi2 itself. The same is true for __builtin_ctz and __ctz{d,s}i2 (which will also make use of a ctz OR clz (clz(x & (~x+1)) is 31 - ctz(x)) instruction if available).
Is this a regression from 5.0.0? I'm nervous about merging this to 6.0.0 since we're late in the process, and it's not a completely straight-forward fix. Unless it's really fixing a regression from the previous release, it might have to wait until 6.0.1.
I agree, this is not a regression and should go in 6.0.1, assuming D43146 has landed by then. I see no point in getting only a partial workaround in 6.0.