[PATCH] D104945: [WebAssembly] Added initial type checker to MC Assembler

[Sam Clegg via Phabricator via llvm-commits]

sbc100 added inline comments.


================
Comment at: lld/test/wasm/Inputs/call-ret32.s:17
   .size ret32_address, 4
 
----------------
Remove extra newline too?


================
Comment at: lld/test/wasm/bsymbolic.s:102
 
+.globl bar
 .globl foo
----------------
Is this change needed here or just for consistency?    If its needed why does declaring the linkage type as global up front help the validator?


================
Comment at: lld/test/wasm/gc-imports.s:18
 _start:
-  .functype _start () -> ()
+  .functype _start () -> (i64)
   call used_undef_function
----------------
Probably best to leave _start void->void as that is in some sense part of the ABI


================
Comment at: lld/test/wasm/relocatable-comdat.s:11
+        .functype _start () -> (i32)
+        i32.const 0
         i32.load foo
----------------
Ditto, leave sig alone and just add drop maybe?


================
Comment at: lld/test/wasm/relocation-bad-tls.s:6
 _start:
-  .functype _start () -> ()
+  .functype _start () -> (i32, i32)
   i32.const foo at TLSREL
----------------
Ditto


================
Comment at: lld/test/wasm/shared64.s:82
 get_func_address:
-  .functype get_func_address () -> (i32)
+  .functype get_func_address () -> (i64)
   global.get func_external at GOT
----------------
Yup.. good catch.  


================
Comment at: lld/test/wasm/signature-mismatch-relocatable.s:15
 _start:
-  .functype _start () -> ()
+  .functype _start () -> (i32)
   i32.const 1
----------------
Ditto.


================
Comment at: llvm/test/CodeGen/WebAssembly/global-get.ll:76
 ; CHECK-NOT: .global i32_external_used
-; CHECK: .globaltype i32_external_used, i32
+; CHECK-NOT: .globaltype i32_external_used, i32
 ; CHECK-NOT: i32_external_used:
----------------
This seems like i32_external_used should have some kind of type, no?    I guess we should look at the FIXME here before landing.


================
Comment at: llvm/test/MC/WebAssembly/basic-assembly.s:59
-    block       f32
-    block       f64
     block       () -> (i32, i32)
----------------
Did we not loose coverage of these different block types here?


================
Comment at: llvm/test/MC/WebAssembly/basic-assembly.s:114
     global.get  __stack_pointer
+    global.set  __stack_pointer
     end_function
----------------
Why this?  Why not just drop?


================
Comment at: llvm/test/MC/WebAssembly/globals.s:26
   global.set global3
+  local.get 3
   global.set global4
----------------
Nice!


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

https://reviews.llvm.org/D104945