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