[PATCH] D78662: [builtins] Support architectures with 16-bit int

[Ayke via Phabricator via llvm-commits]

aykevl added a comment.

In D78662#2000428 <https://reviews.llvm.org/D78662#2000428>, @efriedma wrote:

> > In summary: I think using int is fine as LLVM internally passes an i8
>
> That doesn't seem quite right.  Consider, for example, `long long a(long long a, int b) { return a << (char)(b); }`.  To generate correct code, you need to clear the high bits of b.  If we just model the argument as an anyext i8, we'll skip clearing those bits, and the compiler-rt C implementations will have undefined behavior.  It looks like LLVM does actually extend the value to "int", though.  (It doesn't choose between sign/zero extend consistently, but it doesn't actually matter here).


Interesting, where did you find this?
I tried what would happen on AVR, and it doesn't seem to extend the integer. This could be a backend bug, though.
https://llvm.godbolt.org/z/6RgqVi

  define i32 @shift8(i32 %a, i8 %b) {
    %b32 = zext i8 %b to i32
    %result = lshr i32 %a, %b32
    ret i32 %result
  }



  shift8:
          call    __lshrsi3
          ret

In any case, this patch shouldn't have changed any of that as there was only a change from `si_int` to `int`, and they were equivalent before.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78662