MaskRay added inline comments.
================ Comment at: compiler-rt/lib/builtins/fp_lib.h:49 -static __inline int rep_clz(rep_t a) { return __builtin_clz(a); } +static __inline int rep_clz(rep_t a) { return clzsi(a); } ---------------- atrosinenko wrote: > MaskRay wrote: > > Why is this needed? > First of all, I will compile the examples with gcc because right now I most > probably don't have a TI sysroot for MSP430 compatible with the mainline > clang and testing with my locally patched llvm+clang seems even less correct > than with gcc. > > The problem is that the definition, according to the [gcc > documentation](https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html) is > `int __builtin_clz (unsigned int x)`. This builtin is defined with the > "default size" `int` argument, not `uint32_t` or `int32_t`. Yes, there are > `int`s in the specifications that are not exactly `int` but something > "expected to be int" actually expressed via machine modes. But here it looks > like it is actually an int: > > ```lang=cpp,name=clzsi_test.c > #include <stdio.h> > #include <stdint.h> > > typedef uint32_t rep_t; > > static int rep_clz(rep_t a) { return __builtin_clz(a); } > > int main() > { > printf("sizeof(int) = %d\n", (int)sizeof(int)); > printf("first: %d\n", rep_clz(0x1000)); > printf("second: %d\n", rep_clz(0x10000)); > printf("third: %d\n", rep_clz(0x100000)); > } > ``` > > When compiled and launched on x86_64 Linux host, it prints the expected > results: > > ``` > $ gcc -Wall clzsi_test.c -o clzsi_test && ./clzsi_test > sizeof(int) = 4 > first: 19 > second: 15 > third: 11 > ``` > Let's compile and run it with > ``` > $ $sysroot/bin/msp430-elf-gcc --version > msp430-elf-gcc (Mitto Systems Limited - msp430-gcc 8.3.1.25) 8.3.1 > Copyright (C) 2018 Free Software Foundation, Inc. > This is free software; see the source for copying conditions. There is NO > warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. > ``` > > Then we get the following (the `-msim` option is necessary to run the > compiled binary with `msp430-elf-run` simulator): > > ``` > $ $sysroot/bin/msp430-elf-gcc -msim -Wall clzsi_test.c -o clzsi_test && > $sysroot/bin/msp430-elf-run ./clzsi_test > sizeof(int) = 2 > first: 3 > second: 16 > third: 16 > ``` > > So, these are expected results for `clz(i16 arg)` (with the third line being > an UB at all) but not the results expected for `rep_t`-sized argument. > //(When I comment out the third `printf`, other output that now is expected > to be UB-free remains unchanged)// > > When I change `rep_clz` to use `__builtin_clzl`, the output on MSP430 > simulator differs from the host one just in the `sizeof...` line, as expected. > > ``` > sizeof(int) = 2 > first: 19 > second: 15 > third: 11 > ``` > > Of course, now it will output not the expected values on x86_64, so using > `clzsi` instead of a hardcoded name. Where is this clzsi defined? It seems that clzsi is provided by either `__builtin_clz` or `__builtin_clzl`, but the desired version is a customized one. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81408/new/ https://reviews.llvm.org/D81408 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits