[PATCH] D66255: [WebAssembly] Correctly handle va_arg of zero-sized structures

[Guanzhong Chen via Phabricator via cfe-commits]

quantum added inline comments.


================
Comment at: clang/test/CodeGen/wasm-varargs.c:104
+
+struct S test_zero_size_struct(char *fmt, ...) {
+  va_list va;
----------------
dschuff wrote:
> This should maybe be called "test_empty_struct" since its size is actually 1 and not zero.
Done.


================
Comment at: clang/test/CodeGen/wasm-varargs.c:115
+
+// CHECK: define void @test_zero_size_struct([[STRUCT_S:%[^,=]+]]*{{.*}} noalias sret [[AGG_RESULT:%.*]], i8*{{.*}} %fmt, ...) {{.*}} {
+// CHECK:   [[FMT_ADDR:%[^,=]+]] = alloca i8*, align 4
----------------
dschuff wrote:
> could use CHECK_LABEL here and above.
Can't because the line has match patterns.


================
Comment at: clang/test/CodeGen/wasm-varargs.c:125
+// CHECK:   store i8* [[ARGP_NEXT]], i8** [[VA]], align 4
+// CHECK:   [[R0:%[^,=]+]] = bitcast i8* [[ARGP_CUR]] to [[STRUCT_Z]]*
+// CHECK:   [[R1:%[^,=]+]] = bitcast [[STRUCT_Z]]* [[U]] to i8*
----------------
dschuff wrote:
> It looks like most of the output (up to here) is boilerplate for va_arg setup and duplicates the CHECKs above. It might be more clear if we just omitted the duplicated stuff and only CHECK for the things that are different from the above test.
We can't remove the duplicate checks for the second argument because the broken code resulted from the second argument being incorrectly read when the first argument is a blank structure.

Instead, I am changing the whole test to use `CHECK-NEXT` so we can be sure the exact generated code is correct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66255