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

[Ayke via Phabricator via llvm-commits]

aykevl marked an inline comment as done.
aykevl added a comment.

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

> I was trying with msp430; I had trouble getting a testcase to build with AVR (I forget why).  I guess AVR actually behaves differently: it doesn't respect "zeroext" markings on arguments, unlike most targets.
>
> Given that, there's probably a bug we need to fix, in either the AVR backend or type legalization.


I see. There is definitely a chance that AVR doesn't respect `zeroext`. I know there are still bugs in the backend that haven't been fixed yet. So far, ABI compatibility bugs haven't been high on my priority list, as long as LLVM-compiled code works with other LLVM-compiled code.



================
Comment at: compiler-rt/lib/builtins/udivmoddi4.c:85
       }
       return n.s.high >> __builtin_ctz(d.s.high);
     }
----------------
uabelho wrote:
> Did you consider changing this call into the new macro ctzsi?
> 
> Our downstream target also has ints that are 16 bits wide and we've had some downstream changes somewhat similar to this patch, but then we also changed this call (and another one to __builtin_ctz in this file) to __builtin_ctzl.
I only changed the places that needed changes to get the tests to pass. This file should have been tested together with all other builtins so I think it works on AVR. Of course, if it needs to be changed for your target feel free to send a patch :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78662