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
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to