[WebAssembly] Always emit functype directives for defined functions

[asb]

Summary

This addresses bug https://github.com/llvm/llvm-project/issues/54022. For now this means that defined functions will have two .functype directives emitted. Given discussion in that bug has suggested interest
in moving towards using something other than .functype to mark the beginning of a function (which would, as a side-effect, solve this issue), this patch doesn't attempt to avoid that duplication.

Some test cases that used CHECK-LABEL: foo rather than CHECK-LABEL: foo: are broken by this change. This patch updates those test cases to always have a colon at the end of the CHECK-LABEL string.

[asb]

Fix whitespace+comment issues in the functype-emission test case update.

[sbc100]

llvm/test/CodeGen/WebAssembly/byval.ll

24

Can you just change this to byval_arg: instead? (i.e. add a colon)

llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll

21

Ditto.. i hope this isn't actually needed.

[asb]

Update CHECK-LABEL lines by adding a colon at the end, rather than awkwardly skipping over functype declarations with CHECK: .section.

llvm/test/CodeGen/WebAssembly/byval.ll

24

That is a much better solution to the problem - thanks!

[aardappel]

Nice.. not sure why at the time I thought this was sufficient to do for external decls only..

llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.h

public:
 
   MVT getRegType(unsigned RegNo) const;
   std::string regToString(const MachineOperand &MO);
   WebAssemblyTargetStreamer *getTargetStreamer();
   MCSymbolWasm *getMCSymbolForFunction(const Function *F, bool EnableEmEH,
                                        wasm::WasmSignature *Sig,
                                        bool &InvokeDetected);
   MCSymbol *getOrCreateWasmSymbol(StringRef Name);
-  void emitExternalDecls(const Module &M);
+  void emitDecls(const Module &M);
 };
 
 } // end namespace llvm
 
 #endif

llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp

void WebAssemblyAsmPrinter::emitSymbolType(const MCSymbolWasm *Sym) {
   case wasm::WASM_SYMBOL_TYPE_TABLE:
     getTargetStreamer()->emitTableType(Sym);
     break;
   default:
     break; // We only handle globals, tags and tables here
   }
 }
 
-void WebAssemblyAsmPrinter::emitExternalDecls(const Module &M) {
+void WebAssemblyAsmPrinter::emitDecls(const Module &M) {
   if (signaturesEmitted)
     return;
   signaturesEmitted = true;
 
   // Normally symbols for globals get discovered as the MI gets lowered,
   // but we need to know about them ahead of time. This will however,
   // only find symbols that have been used. Unused symbols from globals will
   // not be found here.
if (!Sym->isDefined())
       emitSymbolType(Sym);
   }
 
   DenseSet<MCSymbol *> InvokeSymbols;
   for (const auto &F : M) {
     if (F.isIntrinsic())
       continue;
 
-    // Emit function type info for all undefined functions
-    if (F.isDeclarationForLinker()) {
-      SmallVector<MVT, 4> Results;
-      SmallVector<MVT, 4> Params;
-      computeSignatureVTs(F.getFunctionType(), &F, F, TM, Params, Results);
-      // At this point these MCSymbols may or may not have been created already
-      // and thus also contain a signature, but we need to get the signature
-      // anyway here in case it is an invoke that has not yet been created. We
-      // will discard it later if it turns out not to be necessary.
-      auto Signature = signatureFromMVTs(Results, Params);
-      bool InvokeDetected = false;
-      auto *Sym = getMCSymbolForFunction(
-          &F, WebAssembly::WasmEnableEmEH || WebAssembly::WasmEnableEmSjLj,
-          Signature.get(), InvokeDetected);
+    // Emit function type info for all functions. This will emit duplicate
+    // information for defined functions (which already have function type
+    // info emitted alongside their definition), but this is necessary in
+    // order to enable the single-pass WebAssemblyAsmTypeCheck to succeed.
 
-      // Multiple functions can be mapped to the same invoke symbol. For
-      // example, two IR functions '__invoke_void_i8*' and '__invoke_void_i32'
-      // are both mapped to '__invoke_vi'. We keep them in a set once we emit an
-      // Emscripten EH symbol so we don't emit the same symbol twice.
-      if (InvokeDetected && !InvokeSymbols.insert(Sym).second)
-        continue;
 
-      Sym->setType(wasm::WASM_SYMBOL_TYPE_FUNCTION);
-      if (!Sym->getSignature()) {
-        Sym->setSignature(Signature.get());
-        addSignature(std::move(Signature));
-      } else {
-        // This symbol has already been created and had a signature. Discard it.
-        Signature.reset();
-      }
 
-      getTargetStreamer()->emitFunctionType(Sym);
 
-      if (F.hasFnAttribute("wasm-import-module")) {
-        StringRef Name =
-            F.getFnAttribute("wasm-import-module").getValueAsString();
-        Sym->setImportModule(storeName(Name));
-        getTargetStreamer()->emitImportModule(Sym, Name);
-      }
-      if (F.hasFnAttribute("wasm-import-name")) {
-        // If this is a converted Emscripten EH/SjLj symbol, we shouldn't use
-        // the original function name but the converted symbol name.
-        StringRef Name =
-            InvokeDetected
-                ? Sym->getName()
-                : F.getFnAttribute("wasm-import-name").getValueAsString();
-        Sym->setImportName(storeName(Name));
-        getTargetStreamer()->emitImportName(Sym, Name);
-      }
-    }
 
     if (F.hasFnAttribute("wasm-export-name")) {
       auto *Sym = cast<MCSymbolWasm>(getSymbol(&F));
       StringRef Name = F.getFnAttribute("wasm-export-name").getValueAsString();
       Sym->setExportName(storeName(Name));
       getTargetStreamer()->emitExportName(Sym, Name);
     }
   }
 }
 
 void WebAssemblyAsmPrinter::emitEndOfAsmFile(Module &M) {
   // This is required to emit external declarations (like .functypes) when
   // no functions are defined in the compilation unit and therefore,
-  // emitExternalDecls() is not called until now.
-  emitExternalDecls(M);
+  // emitDecls() is not called until now.
+  emitDecls(M);
 
   // When a function's address is taken, a TABLE_INDEX relocation is emitted
   // against the function symbol at the use site.  However the relocation
   // doesn't explicitly refer to the table.  In the future we may want to
   // define a new kind of reloc against both the function and the table, so
   // that the linker can see that the function symbol keeps the table alive,
   // but for now manually mark the table as live.
   for (const auto &F : M) {
for (auto &F : EmittedFeatures) {
     OutStreamer->emitULEB128IntValue(F.Name.size());
     OutStreamer->emitBytes(F.Name);
   }
 
   OutStreamer->PopSection();
 }
 
 void WebAssemblyAsmPrinter::emitConstantPool() {
-  emitExternalDecls(*MMI->getModule());
+  emitDecls(*MMI->getModule());
   assert(MF->getConstantPool()->getConstants().empty() &&
          "WebAssembly disables constant pools");
 }
 
 void WebAssemblyAsmPrinter::emitJumpTableInfo() {
   // Nothing to do; jump tables are incorporated into the instruction stream.
 }
 

llvm/test/CodeGen/WebAssembly/byval.ll

 
 declare void @ext_func(%SmallStruct*)
 declare void @ext_func_empty(%EmptyStruct* byval(%EmptyStruct))
 declare void @ext_byval_func(%SmallStruct* byval(%SmallStruct))
 declare void @ext_byval_func_align8(%SmallStruct* byval(%SmallStruct) align 8)
 declare void @ext_byval_func_alignedstruct(%AlignedStruct* byval(%AlignedStruct))
 declare void @ext_byval_func_empty(%EmptyStruct* byval(%EmptyStruct))
 
+; Skip past the functype directives, which interfere with the CHECK-LABEL
+; matches.
+;
+; CHECK-LABEL: .section
+
 ; CHECK-LABEL: byval_arg
 define void @byval_arg(%SmallStruct* %ptr) {
  ; CHECK: .functype byval_arg (i32) -> ()
  ; Subtract 16 from SP (SP is 16-byte aligned)
  ; CHECK-NEXT: global.get $push[[L2:.+]]=, __stack_pointer
  ; CHECK-NEXT: i32.const $push[[L3:.+]]=, 16
  ; CHECK-NEXT: i32.sub $push[[L11:.+]]=, $pop[[L2]], $pop[[L3]]
  ; Ensure SP is stored back before the call
  ; CHECK-NEXT: local.tee $push[[L10:.+]]=, $[[SP:.+]]=, $pop[[L11]]{{$}}

llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll

 ; REQUIRES: asserts
 ; RUN: llc < %s -disable-wasm-fallthrough-return-opt -wasm-disable-explicit-locals -wasm-keep-registers -disable-block-placement -verify-machineinstrs -fast-isel=false -machine-sink-split-probability-threshold=0 -cgp-freq-ratio-to-skip-merge=1000 -wasm-enable-eh -exception-model=wasm -mattr=+exception-handling | FileCheck %s
 ; RUN: llc < %s -disable-wasm-fallthrough-return-opt -disable-block-placement -verify-machineinstrs -fast-isel=false -machine-sink-split-probability-threshold=0 -cgp-freq-ratio-to-skip-merge=1000 -wasm-enable-eh -exception-model=wasm -mattr=+exception-handling
 ; RUN: llc < %s -O0 -disable-wasm-fallthrough-return-opt -wasm-disable-explicit-locals -wasm-keep-registers -verify-machineinstrs -wasm-enable-eh -exception-model=wasm -mattr=+exception-handling | FileCheck %s --check-prefix=NOOPT
 ; RUN: llc < %s -disable-wasm-fallthrough-return-opt -wasm-disable-explicit-locals -wasm-keep-registers -disable-block-placement -verify-machineinstrs -fast-isel=false -machine-sink-split-probability-threshold=0 -cgp-freq-ratio-to-skip-merge=1000 -wasm-enable-eh -exception-model=wasm -mattr=+exception-handling -wasm-disable-ehpad-sort -stats 2>&1 | FileCheck %s --check-prefix=NOSORT
 ; RUN: llc < %s -disable-wasm-fallthrough-return-opt -disable-block-placement -verify-machineinstrs -fast-isel=false -machine-sink-split-probability-threshold=0 -cgp-freq-ratio-to-skip-merge=1000 -wasm-enable-eh -exception-model=wasm -mattr=+exception-handling -wasm-disable-ehpad-sort | FileCheck %s --check-prefix=NOSORT-LOCALS
 
 target triple = "wasm32-unknown-unknown"
 
 @_ZTIi = external constant i8*
 @_ZTId = external constant i8*
 
 %class.Object = type { i8 }
 %class.MyClass = type { i32 }
 
+; Skip past the functype directives, which interfere with the CHECK-LABEL
+; matches.
+;
+; CHECK-LABEL: .section
+; NOOPT-LABEL: .section
+; NOSORT-LABEL: .section
+
 ; Simple test case with two catch clauses
 ;
 ; void foo();
 ; void test0() {
 ;   try {
 ;     foo();
 ;   } catch (int) {
 ;   } catch (double) {

llvm/test/CodeGen/WebAssembly/function-pointer64.ll

entry:
   ret void
 }
 
 @fptr = global void (i32)* @bar
 
 ; For simplicity (and compatibility with UB C/C++ code) we keep all types
 ; of pointers the same size, so function pointers (which are 32-bit indices
 ; in Wasm) are represented as 64-bit until called.
+;
+; Skip past the functype directives at the head of the output.
+; CHECK-LABEL: .section
 
 ; CHECK:      .functype foo (i64) -> ()
 ; CHECK-NEXT: i32.const 1
 ; CHECK-NEXT: local.get 0
 ; CHECK-NEXT: i32.wrap_i64
 ; CHECK-NEXT: call_indirect (i32) -> ()
 ; REF:        call_indirect __indirect_function_table, (i32) -> ()
 

llvm/test/CodeGen/WebAssembly/functype-emission.ll

 ; RUN: llc -mtriple=wasm32 -verify-machineinstrs < %s | FileCheck %s
 
 ; Demonstrates that appropriate .functype directives are emitted for defined
 ; functions, declared functions, and any libcalls.
 
-; FIXME - functype directives should be emitted for defined functions as well,
-; to ensure they pass the type checker (which requires any .functype to be
-; encountered ahead of a call to that target).
-
 ; CHECK: .functype __unordtf2 (i64, i64, i64, i64) -> (i32)
 ; CHECK: .functype __multi3 (i32, i64, i64, i64, i64) -> ()
+; CHECK: .functype defined_fun_1 (f64) -> (i64)
+; CHECK: .functype defined_fun_2 (f64, i32) -> (i64)
 ; CHECK: .functype declared_fun (i32, f32, i64) -> (i32)
 
 define i64 @defined_fun_1(double %a) {
 ; CHECK-LABEL: defined_fun_1:
 ; CHECK:         .functype defined_fun_1 (f64) -> (i64)
   %1 = call i64 @defined_fun_2(double %a, i32 1)
   ret i64 %1
 }

llvm/test/CodeGen/WebAssembly/inlineasm-output-template.ll

 ; RUN: llc -mtriple=wasm32 < %s | FileCheck %s
 
+; Skip past the functype directives, which interfere with the CHECK-LABEL
+; matches.
+;
+; CHECK-LABEL: .section
+
 ; Test that %c works with immediates
 ; CHECK-LABEL: test_inlineasm_c_output_template0
 ; CHECK: #TEST 42
 define dso_local i32 @test_inlineasm_c_output_template0() {
   tail call void asm sideeffect "#TEST ${0:c}", "i"(i32 42)
   ret i32 42
 }
 

llvm/test/CodeGen/WebAssembly/multi-return.ll

 ; RUN: llc < %s -asm-verbose=false -wasm-disable-explicit-locals -wasm-keep-registers | FileCheck %s
 
 target triple = "wasm32-unknown-unknown"
 
 ; Return multiple values, some of which will be legalized into multiple values.
 declare { i64, i128, i192, i128, i64 } @return_multi_multi()
 
+; Skip past the functype directives, which interfere with the CHECK-LABEL
+; matches.
+;
+; CHECK-LABEL: .section
+
 ; Test returning a single value from @return_multi_multi.
 
 define i64 @test0() {
 ; CHECK-LABEL: test0
 ; CHECK: call     return_multi_multi
 ; CHECK: i64.load $[[RV:[0-9]+]]=, 8(${{[0-9]+}})
 ; CHECK: local.copy $push8=, $[[RV]]
   %t0 = call { i64, i128, i192, i128, i64 } @return_multi_multi()

llvm/test/CodeGen/WebAssembly/stack-protector.ll

 ; RUN: llc -verify-machineinstrs -mtriple=wasm32-unknown-unknown < %s | FileCheck -check-prefix=WASM32 %s
 
 @"\01LC" = internal constant [11 x i8] c"buf == %s\0A\00"    ; <[11 x i8]*> [#uses=1]
 
+; Skip past the functype directives, which interfere with the CHECK-LABEL
+; matches.
+;
+; WASM32-LABEL: .section
+
 ; WASM32-LABEL: test
 ; WASM32:      i32.load        28
 ; WASM32:      br_if           0
 ; WASM32:      call __stack_chk_fail
 ; WASM32-NEXT: unreachable
 
 define void @test(i8* %a) nounwind ssp {
 entry:

llvm/test/MC/WebAssembly/stack-ptr-mclower.ll

 entry:
   alloca i32, align 4
   ret void
 }
 
 ; CHECK:              .text
 ; CHECK-NEXT:         .file   "stack-ptr-mclower.ll"
 ; CHECK-NEXT:         .globaltype     __stack_pointer, [[PTR]]
+; CHECK-NEXT:         .functype bar () -> ()
+; CHECK-NEXT:         .functype foo () -> ()
+
 ; CHECK-NEXT:         .section        .text.bar,"",@
 ; CHECK-NEXT:         .hidden bar
 ; CHECK-NEXT:         .globl  bar
 ; CHECK-NEXT:         .type   bar,@function
 ; CHECK-NEXT: bar:
 ; CHECK-NEXT:         .functype       bar () -> ()
 ; CHECK-NEXT:         end_function
 ; CHECK-NEXT: .Lfunc_end0: