[PATCH] D52914: [WebAssembly] Added test for inline assembly roundtrip.

Sam Clegg via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 4 17:06:56 PDT 2018


sbc100 added a comment.

Sweet!

I kind of prefer the test to be as minimal as possible even if derived from c source, but perhaps thats overly pedantic.



================
Comment at: test/CodeGen/WebAssembly/inline-asm-roundtrip.ll:20
+
+source_filename = "test.c"
+target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
----------------
drop this line


================
Comment at: test/CodeGen/WebAssembly/inline-asm-roundtrip.ll:22
+target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
+target triple = "wasm32-unknown-unknown-wasm"
+
----------------
Drop the "-wasm": here and in the command line, like the other tests in this dir


================
Comment at: test/CodeGen/WebAssembly/inline-asm-roundtrip.ll:37
+
+define hidden i32 @main(i32 %argc, i8** nocapture readnone %argv) local_unnamed_addr #0 {
+entry:
----------------
Hmm.. is main compiled by clang as hidden?   In any case, you can drop that for the test


================
Comment at: test/CodeGen/WebAssembly/inline-asm-roundtrip.ll:53
+!1 = !{!"clang version 8.0.0 (trunk 343492) (llvm/trunk 343501)"}
+!2 = !{i32 117, i32 140, i32 165, i32 184}
----------------
Can you trip all this attribute stuff to keep the test nice and small?


Repository:
  rL LLVM

https://reviews.llvm.org/D52914





More information about the llvm-commits mailing list