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

Mon Jun 12 16:58:53 PDT 2023

sbc100 added a comment.

In D151820#4415754 <https://reviews.llvm.org/D151820#4415754>, @dschuff wrote:

>> 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?

Presumably this change of change makes most sense in the emscripten ChangeLog right?   We don't tend to document emscripten-specific changes in the llvm release notes do we?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151820