[PATCH] D41941: [WebAssembly] Change int_fast16_t to 32-bit

Nicholas Wilson via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 11 10:20:28 PST 2018


ncw 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.
----------------
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".


Repository:
  rC Clang

https://reviews.llvm.org/D41941





More information about the cfe-commits mailing list