dschuff added inline comments.
================
Comment at: lib/Basic/Targets/WebAssembly.h:108
IntType getLeastIntTypeByWidth(unsigned BitWidth, bool IsSigned) const final
{
- // WebAssembly uses long long for int_least64_t and int_fast64_t.
- return BitWidth == 64
+ // WebAssembly uses long long for int_least64_t and int_fast64_t, and int
+ // for int_least16_t and int_fast16_t.
----------------
ncw wrote:
> dschuff wrote:
> > ncw wrote:
> > > ncw wrote:
> > > > dschuff wrote:
> > > > > I think we want least16_t to still be short, no? We do still support
> > > > > 16-bit shorts, so my interpretation is that the smallest type with
> > > > > width of at least 16 should still be 16.
> > > > ...is there a way to do that? I couldn't find any other archs that do
> > > > it; it seems like the stdint.h that Clang provides requires least16_t
> > > > to match fast16_t. I copied this from the AVR target, although maybe
> > > > that doesn't support 16-bit at all.
> > > Sorry, now I see that AVR uses int because it has 16-bit ints...
> > >
> > > There isn't any existing Clang target that uses 32-bit for fast16_t, so
> > > maybe it's currently not possible within Clang's framework (or at least,
> > > not without also fiddling with least16_t).
> > > `lib/Frontend/InitPreprocessor.cpp` hardcodes some logic with sets them
> > > to be the same.
> > >
> > > I can abandon this review if that's not acceptable collateral damage
> > > (probably not, on reflection) - or could tweak InitPreprocessor.cpp and
> > > stdint.h to be more flexible (might need more review if you don't "own"
> > > those files?)
> > I think it's worth trying to fix InitPreprocessor.cpp/stdint.h to remove
> > the assumption that those types are the same. We'll need to get broader
> > review, so it will be slower than if we were just changing our own files,
> > but that's OK. If you're up for doing that I'd be happy to help you find
> > reviewers, or otherwise I can take a shot at it; now is a good time since
> > we're taking a closer look at our ABIs more generally anyway.
> uhh... if you could take a look that would be great! I've only got a limited
> Wasm "budget" from my employer, and have to get back to customer work for
> most of the rest of the week.
>
> Was there an issue I seem to remember as well, where Wasm32 was using
> "unsigned int" for __SIZE_TYPE__ instead of "unsigned long".
Yes, @sunfish is switching the wasm and asm.js ABIs to both use unsigned long.
Repository:
rC Clang
https://reviews.llvm.org/D41941
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits