[WebAssembly] Enable main-function signature rewriting for WASI

[sunfish]

Summary

Enable the functionality to rewrite the signature of "int main(void)" functions to "int main(int argc, char *argv[])", so that they can be called by the C startup code, since WebAssembly requires caller and callee signatures to match.

[sbc100]

I tried to enable this by default recently but had to revert: https://reviews.llvm.org/D54117.

I'm fine for this to land in the mean time but you might want to be aware of this issue.

[sunfish]

In D57323#1374325, @sbc100 wrote:

I tried to enable this by default recently but had to revert: https://reviews.llvm.org/D54117.

I'm fine for this to land in the mean time but you might want to be aware of this issue.

The patch here avoids the envp issue by limiting the transformation to the (standard-approved) int main(void) case, and not arbitrary signatures. So if it sees int main(int argc, char *argv[], char *envp[]), it won't do anything.

[sbc100]

In D57323#1374475, @sunfish wrote:

In D57323#1374325, @sbc100 wrote:

I tried to enable this by default recently but had to revert: https://reviews.llvm.org/D54117.

I'm fine for this to land in the mean time but you might want to be aware of this issue.

The patch here avoids the envp issue by limiting the transformation to the (standard-approved) int main(void) case, and not arbitrary signatures. So if it sees int main(int argc, char *argv[], char *envp[]), it won't do anything.

In that case can you just remove the TemporaryWorkarounds flag and always do this transform? That is the behaviour we want and I only reverted my change because of the envp issue.

[sbc100]

I mean with your fix we don't need this to be wasi specific do we?

[sunfish]

In D57323#1374572, @sbc100 wrote:

I mean with your fix we don't need this to be wasi specific do we?

It needs to align with what crt1.o does though, which is specific to the OS. For WASI's crt1.o, my current working theory is to just support the two standard-required cases and not the envp case. But for the unknown OS, we don't know what people might want to link to.

[sbc100]

In D57323#1374661, @sunfish wrote:

In D57323#1374572, @sbc100 wrote:

I mean with your fix we don't need this to be wasi specific do we?

It needs to align with what crt1.o does though, which is specific to the OS. For WASI's crt1.o, my current working theory is to just support the two standard-required cases and not the envp case. But for the unknown OS, we don't know what people might want to link to.

But we already decided we wanted this behaviour and my change only got reverted because it was demoting the 3-arg form to the 2-arg form. Musl's crt0 currently expects the 2-arg form so this would align exactly with what you have done here: promote the 0-arg to 2-arg form but leave the 3-arg form (and all other forms) alone.

[sunfish]

• Remove the target constraint, allowing the main rewrite logic to work on all wasm32 targets.
• Remove the now-unneeded "wasm-temporary-workarounds" flag altogether.
• This uncovered some bugs with wrappers for main when main is only a declaration; fix those as well.

[sunfish]

In D57323#1374682, @sbc100 wrote:

But we already decided we wanted this behaviour and my change only got reverted because it was demoting the 3-arg form to the 2-arg form. Musl's crt0 currently expects the 2-arg form so this would align exactly with what you have done here: promote the 0-arg to 2-arg form but leave the 3-arg form (and all other forms) alone.

Ok, that works for me. And this way we can get rid of the wasm-temporary-workarounds flag completely, which is a nice bonus. Patch updated.

[sbc100]

test/CodeGen/WebAssembly/main-three-args.ll

4 ↗

(On Diff #183988)

Drop the WASI

[kripken]

This broke binaryen2.test_exceptions_white_list_2

e.g. https://logs.chromium.org/logs/wasm/buildbucket/cr-buildbucket.appspot.com/8922985927078952624/+/steps/Execute_emscripten_testsuite__emwasm_/0/stdout

llvm/trunk/lib/Target/WebAssembly/WebAssemblyFixFunctionBitcasts.cpp

 #include "llvm/IR/Operator.h"
 #include "llvm/Pass.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
 using namespace llvm;
 
 #define DEBUG_TYPE "wasm-fix-function-bitcasts"
 
-static cl::opt<bool>
-    TemporaryWorkarounds("wasm-temporary-workarounds",
-                         cl::desc("Apply certain temporary workarounds"),
-                         cl::init(true), cl::Hidden);
-
 namespace {
 class FixFunctionBitcasts final : public ModulePass {
   StringRef getPassName() const override {
     return "WebAssembly Fix Function Bitcasts";
   }
 
   void getAnalysisUsage(AnalysisUsage &AU) const override {
     AU.setPreservesCFG();
LLVM_DEBUG(dbgs() << "CreateWrapper: no wrapper needed: " << F->getName()
                       << "\n");
     Wrapper->eraseFromParent();
     return nullptr;
   }
   LLVM_DEBUG(dbgs() << "CreateWrapper: " << F->getName() << "\n");
   return Wrapper;
 }
 
+// Test whether a main function with type FuncTy should be rewritten to have
+// type MainTy.
+bool ShouldFixMainFunction(FunctionType *FuncTy, FunctionType *MainTy) {
+  // Only fix the main function if it's the standard zero-arg form. That way,
+  // the standard cases will work as expected, and users will see signature
+  // mismatches from the linker for non-standard cases.
+  return FuncTy->getReturnType() == MainTy->getReturnType() &&
+         FuncTy->getNumParams() == 0 &&
+         !FuncTy->isVarArg();
+}
+
 bool FixFunctionBitcasts::runOnModule(Module &M) {
   LLVM_DEBUG(dbgs() << "********** Fix Function Bitcasts **********\n");
 
   Function *Main = nullptr;
   CallInst *CallMain = nullptr;
   SmallVector<std::pair<Use *, Function *>, 0> Uses;
   SmallPtrSet<Constant *, 2> ConstantBCs;
 
   // Collect all the places that need wrappers.
   for (Function &F : M) {
     FindUses(&F, F, Uses, ConstantBCs);
 
     // If we have a "main" function, and its type isn't
     // "int main(int argc, char *argv[])", create an artificial call with it
     // bitcasted to that type so that we generate a wrapper for it, so that
     // the C runtime can call it.
-    if (!TemporaryWorkarounds && !F.isDeclaration() && F.getName() == "main") {
+    if (F.getName() == "main") {
       Main = &F;
       LLVMContext &C = M.getContext();
       Type *MainArgTys[] = {Type::getInt32Ty(C),
                             PointerType::get(Type::getInt8PtrTy(C), 0)};
       FunctionType *MainTy = FunctionType::get(Type::getInt32Ty(C), MainArgTys,
                                                /*isVarArg=*/false);
-      if (F.getFunctionType() != MainTy) {
+      if (ShouldFixMainFunction(F.getFunctionType(), MainTy)) {
         LLVM_DEBUG(dbgs() << "Found `main` function with incorrect type: "
                           << *F.getFunctionType() << "\n");
         Value *Args[] = {UndefValue::get(MainArgTys[0]),
                          UndefValue::get(MainArgTys[1])};
         Value *Casted =
             ConstantExpr::getBitCast(Main, PointerType::get(MainTy, 0));
         CallMain = CallInst::Create(Casted, Args, "call_main");
         Use *UseMain = &CallMain->getOperandUse(2);
bool FixFunctionBitcasts::runOnModule(Module &M) {
   }
 
   // If we created a wrapper for main, rename the wrapper so that it's the
   // one that gets called from startup.
   if (CallMain) {
     Main->setName("__original_main");
     Function *MainWrapper =
         cast<Function>(CallMain->getCalledValue()->stripPointerCasts());
+    delete CallMain;
+    if (Main->isDeclaration()) {
+      // The wrapper is not needed in this case as we don't need to export
+      // it to anyone else.
+      MainWrapper->eraseFromParent();
+    } else {
+      // Otherwise give the wrapper the same linkage as the original main
+      // function, so that it can be called from the same places.
       MainWrapper->setName("main");
       MainWrapper->setLinkage(Main->getLinkage());
       MainWrapper->setVisibility(Main->getVisibility());
-    Main->setLinkage(Function::PrivateLinkage);
-    Main->setVisibility(Function::DefaultVisibility);
-    delete CallMain;
+    }
   }
 
   return true;
 }

llvm/trunk/test/CodeGen/WebAssembly/call.ll

-; RUN: llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt -wasm-keep-registers -wasm-temporary-workarounds=false -mattr=+sign-ext,+simd128 | FileCheck %s
-; RUN: llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt -wasm-keep-registers -fast-isel -fast-isel-abort=1 -wasm-temporary-workarounds=false -mattr=+sign-ext,+simd128 | FileCheck %s
+; RUN: llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt -wasm-keep-registers -mattr=+sign-ext,+simd128 | FileCheck %s
+; RUN: llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt -wasm-keep-registers -fast-isel -fast-isel-abort=1 -mattr=+sign-ext,+simd128 | FileCheck %s
 
 ; Test that basic call operations assemble as expected.
 
 target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
 target triple = "wasm32-unknown-unknown"
 
 declare i32 @i32_nullary()
 declare i32 @i32_unary(i32)

llvm/trunk/test/CodeGen/WebAssembly/function-bitcasts-varargs.ll

-; RUN: llc < %s -asm-verbose=false -wasm-temporary-workarounds=false -wasm-keep-registers | FileCheck %s
+; RUN: llc < %s -asm-verbose=false -wasm-keep-registers | FileCheck %s
 
 ; Test that function pointer casts casting away varargs are replaced with
 ; wrappers.
 
 target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
 target triple = "wasm32-unknown-unknown"
 
 define void @callWithArgs() {

llvm/trunk/test/CodeGen/WebAssembly/function-bitcasts.ll

-; RUN: llc < %s -asm-verbose=false -wasm-disable-explicit-locals -wasm-keep-registers -enable-emscripten-cxx-exceptions -wasm-temporary-workarounds=false | FileCheck %s
+; RUN: llc < %s -asm-verbose=false -wasm-disable-explicit-locals -wasm-keep-registers -enable-emscripten-cxx-exceptions | FileCheck %s
 
 ; Test that function pointer casts are replaced with wrappers.
 
 target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
 target triple = "wasm32-unknown-unknown"
 
 declare void @has_i32_arg(i32)
 declare void @has_struct_arg({i32})

llvm/trunk/test/CodeGen/WebAssembly/main-declaration.ll

-; RUN: llc < %s -asm-verbose=false -wasm-temporary-workarounds=false | FileCheck %s
+; RUN: llc < %s -asm-verbose=false | FileCheck %s
 
 ; Test main functions with alternate signatures.
 
 target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
 target triple = "wasm32-unknown-unknown"
 
-declare void @main()
+declare i32 @main()
 
-define void @foo() {
-  call void @main()
-  ret void
+define i32 @foo() {
+  %t = call i32 @main()
+  ret i32 %t
 }
 
-; CHECK-NOT:   __original_main
 ; CHECK-LABEL: foo:
-; CHECK-NEXT:    .functype foo () -> ()
-; CHECK-NEXT:    call main@FUNCTION
+; CHECK-NEXT:    .functype foo () -> (i32)
+; CHECK-NEXT:    call __original_main@FUNCTION
 ; CHECK-NEXT:    end_function
-; CHECK-NOT:   __original_main

llvm/trunk/test/CodeGen/WebAssembly/main-no-args.ll

-; RUN: llc < %s -asm-verbose=false -wasm-temporary-workarounds=false | FileCheck %s
+; RUN: llc < %s -asm-verbose=false | FileCheck %s
 
 ; Test main functions with alternate signatures.
 
 target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
 target triple = "wasm32-unknown-unknown"
 
-define void @main() {
-  ret void
+define i32 @main() {
+  ret i32 0
 }
 
-; CHECK-LABEL: .L__original_main:
-; CHECK-NEXT: .functype .L__original_main () -> ()
+; CHECK-LABEL: __original_main:
+; CHECK-NEXT: .functype __original_main () -> (i32)
+; CHECK-NEXT: i32.const 0
 ; CHECK-NEXT: end_function
 
 ; CHECK-LABEL: main:
 ; CHECK-NEXT: .functype main (i32, i32) -> (i32)
-; CHECK:      call .L__original_main@FUNCTION
+; CHECK:      call __original_main@FUNCTION

llvm/trunk/test/CodeGen/WebAssembly/main-three-args.ll

+; RUN: llc < %s -asm-verbose=false | FileCheck %s
+
+; Test that main function with a non-standard third argument is
+; not wrapped.
+
+target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
+target triple = "wasm32-unknown-unknown"
+
+define i32 @main(i32 %a, i8** %b, i8** %c) {
+  ret i32 0
+}
+
+; CHECK-LABEL: main:
+; CHECK-NEXT: .functype main (i32, i32, i32) -> (i32)
+
+; CHECK-NOT: __original_main:

llvm/trunk/test/CodeGen/WebAssembly/main-with-args.ll

-; RUN: llc < %s -asm-verbose=false -wasm-temporary-workarounds=false | FileCheck %s
+; RUN: llc < %s -asm-verbose=false | FileCheck %s
 
 ; Test that main function with expected signature is not wrapped
 
 target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
 target triple = "wasm32-unknown-unknown"
 
 define i32 @main(i32 %a, i8** %b) {
   ret i32 0
 }
 
 ; CHECK-LABEL: main:
 ; CHECK-NEXT: .functype main (i32, i32) -> (i32)
 
 ; CHECK-NOT: __original_main: