[PATCH] D151820: [clang][WebAssembly] Fix __BIGGEST_ALIGNMENT__ under emscripten

Mon Jun 12 16:51:32 PDT 2023

dschuff added a comment.

> As far as I can tell the only way this change could break XNNpack if XNN_ALLOCATION_ALIGNMENT = 8 is wrongly set there... as long as that is the correct value for XNN_ALLOCATION_ALIGNMENT I don't see how this change could break it.  If XNN_ALLOCATION_ALIGNMENT is set wrongly this change might expose that bug.. but it seems correct to me.

yeah, that's actually what my concern is. IIUC as written the code is asking for 8, but it's being masked by our value of BIGGEST_ALIGNMENT.

I suppose we should land this since I think we do want to have it match max_align_t. But it does make me wonder (again) whether our choice of ABI is correct here.
Can you also put something in the emscripten release notes about this?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151820