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