glider marked 3 inline comments as done. glider added inline comments.
================ Comment at: clang/test/CodeGen/PR42672.c:50 + struct { + long long int v1, v2, v3, v4; + } str; ---------------- efriedma wrote: > glider wrote: > > The acceptable size actually depends on the target platform. Not sure we > > can test for all of them, and we'll probably need to restrict this test to > > e.g. x86 > The interesting case for a 32-byte struct would actually be something like > `"=x"(str)`... which currently passes the clang frontend, since 32 is a legal > size for that constraint (although it eventually fails in the backend). Changing "=r" to "=X" indeed makes this particular test pass (there's nothing to fail in the backend, as we don't actually generate instructions that write to memory) I'm however unsure adding a test for "=X" makes any difference, as our patch is irrelevant to this constraint. ================ Comment at: clang/test/CodeGen/PR42672.c:40 + unsigned short first; + unsigned char second; + } str; ---------------- efriedma wrote: > This isn't a three-byte struct; it's a four-byte struct where one of the > bytes is only used for padding. You're right. Making this structure packed yields an error, which conforms to GCC behavior. I've added a test for that as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65234/new/ https://reviews.llvm.org/D65234 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits