[PATCH] D81285: [builtins] Change si_int to int in some helper declarations

[Ayke via Phabricator via cfe-commits]

aykevl added a comment.

I need to recompile LLVM to test this patch so it might take a while.

In D81285#2082517 <https://reviews.llvm.org/D81285#2082517>, @atrosinenko wrote:

> In D81285#2082394 <https://reviews.llvm.org/D81285#2082394>, @aykevl wrote:
>
> > Also note that the libgcc documentation does not always reflect the real world. For example, `__divmodsi4` on AVR libgcc has a very different signature: it returns both the division result and the remainder in registers.
>
>
> Do you mean some special calling convention not used outside the `libgcc`/`clang_rt`? MSP430 has one as well. And I still have to decide how to express for //some of// generic C implementations that they should use that special calling convention on MSP430 without cluttering the sources with `O(target count)` complexity. :) Meanwhile, some hacks do exist for ARM target already.


I don't mean a separate calling convention, although AVR has that as well. Rather, that the builtin has a different function signature. This signature is a bit hard to express in C so you might consider it a different ABI I guess. I implemented it in C by packing the two return values in a single `uint64_t`.



================
Comment at: compiler-rt/lib/builtins/int_lib.h:112
 
-uint32_t __inline __builtin_ctz(uint32_t value) {
+int __inline __builtin_ctz(uint32_t value) {
   unsigned long trailing_zero = 0;
----------------
atrosinenko wrote:
> aykevl wrote:
> > Why `int` and not `native_int` here?
> Just to use more "textually identical" prototype as for an actual `__builtin_ctz` from GCC. On the other hand, such dilemma could be one of arguments against `native_int`.
Yes, that makes sense. It is actually defined with `int` in [the official documentation](https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html), and because this is at the C language level and not at the ABI/target level, it's not one of the "int but really int32_t" types.

That said, I think the parameter should also be `unsigned int` instead of `uint32_t`. And the same goes for the other types below. Although, in practice, it doesn't really matter as it's all MSVC with 32-bit int everywhere.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81285/new/

https://reviews.llvm.org/D81285