[WebAssembly] Support opaque pointers in FixFunctionBitcasts

[nikic]

Summary

With opaque pointers, no actual bitcasts will be present. Instead, there will be a mismatch between the call FunctionType and the function ValueType. Change the code to collect CallBases specifically (rather than general Uses) and compare these types.

[tlively]

@dschuff, IIRC, we were talking about what opaque pointers would mean for function pointer casts just the other day. Well here's the answer!

llvm/lib/Target/WebAssembly/WebAssemblyFixFunctionBitcasts.cpp

84–86

Why can this logic be removed?

[nikic]

llvm/lib/Target/WebAssembly/WebAssemblyFixFunctionBitcasts.cpp

84–86

I dropped the RAUW call on constants that this is guarding against. With opaque pointers, there is no bitcast to RAUW (the only thing that could be potentially RAUWd is the GlobalAlias case).

I'm not sure if dropping the RAUW call itself is problematic, at least there was no test coverage for why it would be needed. I could imagine something like a function bitcast being called directly in one place, and then also being stored to memory in another place and then called indirectly or something like that? Though that already wouldn't work if there is no direct call and only indirect calls. So I'm not really sure why it was there.

llvm/lib/Target/WebAssembly/WebAssemblyFixFunctionBitcasts.cpp

 
 ModulePass *llvm::createWebAssemblyFixFunctionBitcasts() {
   return new FixFunctionBitcasts();
 }
 
 // Recursively descend the def-use lists from V to find non-bitcast users of
 // bitcasts of V.
 static void findUses(Value *V, Function &F,
-                     SmallVectorImpl<std::pair<Use *, Function *>> &Uses,
-                     SmallPtrSetImpl<Constant *> &ConstantBCs) {
-  for (Use &U : V->uses()) {
-    if (auto *BC = dyn_cast<BitCastOperator>(U.getUser()))
-      findUses(BC, F, Uses, ConstantBCs);
-    else if (auto *A = dyn_cast<GlobalAlias>(U.getUser()))
-      findUses(A, F, Uses, ConstantBCs);
-    else if (U.get()->getType() != F.getType()) {
-      CallBase *CB = dyn_cast<CallBase>(U.getUser());
-      if (!CB)
-        // Skip uses that aren't immediately called
-        continue;
+                     SmallVectorImpl<std::pair<CallBase *, Function *>> &Uses) {
+  for (User *U : V->users()) {
+    if (auto *BC = dyn_cast<BitCastOperator>(U))
+      findUses(BC, F, Uses);
+    else if (auto *A = dyn_cast<GlobalAlias>(U))
+      findUses(A, F, Uses);
+    else if (auto *CB = dyn_cast<CallBase>(U)) {
       Value *Callee = CB->getCalledOperand();
       if (Callee != V)
         // Skip calls where the function isn't the callee
         continue;
-      if (isa<Constant>(U.get())) {
-        // Only add constant bitcasts to the list once; they get RAUW'd
-        auto C = ConstantBCs.insert(cast<Constant>(U.get()));
-        if (!C.second)
-          continue;
-      }
-      Uses.push_back(std::make_pair(&U, &F));
+      if (CB->getFunctionType() == F.getValueType())
+        // Skip uses that are immediately called
+      Uses.push_back(std::make_pair(CB, &F));
     }
   }
 }
 
 // Create a wrapper function with type Ty that calls F (which may have a
 // different type). Attempt to support common bitcasted function idioms:
 //  - Call with more arguments than needed: arguments are dropped
 //  - Call with fewer arguments than needed: arguments are filled in with undef
return FuncTy->getReturnType() == MainTy->getReturnType() &&
          !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;
+  SmallVector<std::pair<CallBase *, Function *>, 0> Uses;
 
   // Collect all the places that need wrappers.
   for (Function &F : M) {
     // Skip to fix when the function is swiftcc because swiftcc allows
     // bitcast type difference for swiftself and swifterror.
     if (F.getCallingConv() == CallingConv::Swift)
       continue;
-    findUses(&F, F, Uses, ConstantBCs);
+    findUses(&F, F, Uses);
 
     // 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 (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 (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(MainTy, Casted, Args, "call_main");
-        Use *UseMain = &CallMain->getOperandUse(2);
-        Uses.push_back(std::make_pair(UseMain, &F));
+        Uses.push_back(std::make_pair(CallMain, &F));
       }
     }
   }
 
   DenseMap<std::pair<Function *, FunctionType *>, Function *> Wrappers;
 
   for (auto &UseFunc : Uses) {
-    Use *U = UseFunc.first;
+    CallBase *CB = UseFunc.first;
     Function *F = UseFunc.second;
-    auto *PTy = cast<PointerType>(U->get()->getType());
-    auto *Ty = dyn_cast<FunctionType>(PTy->getElementType());
-
-    // If the function is casted to something like i8* as a "generic pointer"
-    // to be later casted to something else, we can't generate a wrapper for it.
-    // Just ignore such casts for now.
-    if (!Ty)
-      continue;
+    FunctionType *Ty = CB->getFunctionType();
 
     auto Pair = Wrappers.insert(std::make_pair(std::make_pair(F, Ty), nullptr));
     if (Pair.second)
       Pair.first->second = createWrapper(F, Ty);
 
     Function *Wrapper = Pair.first->second;
     if (!Wrapper)
       continue;
 
-    if (isa<Constant>(U->get()))
-      U->get()->replaceAllUsesWith(Wrapper);
-    else
-      U->set(Wrapper);
+    CB->setCalledOperand(Wrapper);
   }
 
   // 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");
     auto *MainWrapper =
         cast<Function>(CallMain->getCalledOperand()->stripPointerCasts());

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

-; RUN: llc < %s -asm-verbose=false -wasm-disable-explicit-locals -wasm-keep-registers -enable-emscripten-cxx-exceptions | FileCheck %s
+; RUN: llc < %s -asm-verbose=false -wasm-disable-explicit-locals -wasm-keep-registers -enable-emscripten-cxx-exceptions | FileCheck %s --check-prefixes=CHECK,TYPED
+; RUN: llc < %s -asm-verbose=false -wasm-disable-explicit-locals -wasm-keep-registers -enable-emscripten-cxx-exceptions -force-opaque-pointers | FileCheck %s --check-prefixes=CHECK,OPAQUE
 
 ; Test that function pointer casts are replaced with wrappers.
 
+; The TYPED and OPAQUE prefixes only differ in function ordering.
+
 target triple = "wasm32-unknown-unknown"
 
 define void @has_i32_arg(i32) {
 entry:
   ret void
 }
 
 declare void @has_struct_arg({i32})
 declare i32 @has_i32_ret()
 declare void @vararg(...)
 declare void @plain(i32)
 
 declare void @foo0()
 declare void @foo1()
 declare void @foo2()
 declare void @foo3()
 
 ; CHECK-LABEL: test:
-; CHECK:      call        .Lhas_i32_arg_bitcast.2{{$}}
-; CHECK-NEXT: call        .Lhas_i32_arg_bitcast.2{{$}}
+; TYPED:      call        .Lhas_i32_arg_bitcast.2{{$}}
+; TYPED-NEXT: call        .Lhas_i32_arg_bitcast.2{{$}}
+; OPAQUE:      call       .Lhas_i32_arg_bitcast{{$}}
+; OPAQUE-NEXT: call       .Lhas_i32_arg_bitcast{{$}}
 ; CHECK-NEXT: call        .Lhas_i32_ret_bitcast{{$}}
 ; CHECK-NEXT: call        $drop=, has_i32_ret
 ; CHECK-NEXT: i32.const   $push[[L0:[0-9]+]]=, 0
 ; CHECK-NEXT: call        .Lfoo0_bitcast, $pop[[L0]]{{$}}
 ; CHECK-NEXT: i32.const   $push[[L1:[0-9]+]]=, 0
 ; CHECK-NEXT: call        .Lfoo0_bitcast, $pop[[L1]]{{$}}
 ; CHECK-NEXT: i32.const   $push[[L2:[0-9]+]]=, 0
 ; CHECK-NEXT: call        .Lfoo0_bitcast, $pop[[L2]]{{$}}
entry:
   ret void
 }
 
 ; Calling aliases should also generate a wrapper
 
 @alias_i32_arg = weak hidden alias void (i32), void (i32)* @has_i32_arg
 
 ; CHECK-LABEL: test_alias:
-; CHECK: call    .Lhas_i32_arg_bitcast.2
+; TYPED: call    .Lhas_i32_arg_bitcast.2
+; OPAQUE: call   .Lhas_i32_arg_bitcast
 define void @test_alias() {
 entry:
   call void bitcast (void (i32)* @alias_i32_arg to void ()*)()
   ret void
 }
 
 
 ; CHECK-LABEL: test_structs:
-; CHECK: call     .Lhas_i32_arg_bitcast.1, $pop{{[0-9]+}}, $pop{{[0-9]+$}}
-; CHECK: call     .Lhas_i32_arg_bitcast, $0, $pop2
+; TYPED: call     .Lhas_i32_arg_bitcast.1, $pop{{[0-9]+}}, $pop{{[0-9]+$}}
+; TYPED: call     .Lhas_i32_arg_bitcast, $0, $pop2
+; OPAQUE: call    .Lhas_i32_arg_bitcast.2, $pop{{[0-9]+}}, $pop{{[0-9]+$}}
+; OPAQUE: call    .Lhas_i32_arg_bitcast.1, $0, $pop2
 ; CHECK: call     .Lhas_struct_arg_bitcast{{$}}
 define void @test_structs() {
 entry:
   call void bitcast (void (i32)* @has_i32_arg to void (i32, {i32})*)(i32 5, {i32} {i32 6})
   call {i32, i64} bitcast (void (i32)* @has_i32_arg to {i32, i64} (i32)*)(i32 7)
   call void bitcast (void ({i32})* @has_struct_arg to void ()*)()
   ret void
 }

 
 ; CHECK-LABEL: test_invoke:
 ; CHECK:      i32.const   $push[[L1:[0-9]+]]=, call_func{{$}}
 ; CHECK-NEXT: i32.const   $push[[L0:[0-9]+]]=, has_i32_ret{{$}}
 ; CHECK-NEXT: call        invoke_vi, $pop[[L1]], $pop[[L0]]{{$}}
 ; CHECK:      i32.const   $push[[L3:[0-9]+]]=, call_func{{$}}
 ; CHECK-NEXT: i32.const   $push[[L2:[0-9]+]]=, has_i32_arg{{$}}
 ; CHECK-NEXT: call        invoke_vi, $pop[[L3]], $pop[[L2]]{{$}}
-; CHECK:      i32.const   $push[[L4:[0-9]+]]=, .Lhas_i32_arg_bitcast.2{{$}}
+; TYPED:      i32.const   $push[[L4:[0-9]+]]=, .Lhas_i32_arg_bitcast.2{{$}}
+; OPAQUE:     i32.const   $push[[L4:[0-9]+]]=, .Lhas_i32_arg_bitcast{{$}}
 ; CHECK-NEXT: call        invoke_v, $pop[[L4]]{{$}}
 declare i32 @personality(...)
 define void @test_invoke() personality i32 (...)* @personality {
 entry:
   invoke void @call_func(i32 ()* @has_i32_ret)
           to label %cont unwind label %lpad
 
 cont:
   invoke void @call_func(i32 ()* bitcast (void (i32)* @has_i32_arg to i32 ()*))
           to label %cont2 unwind label %lpad
 
 cont2:
   invoke void bitcast (void (i32)* @has_i32_arg to void ()*)()
           to label %end unwind label %lpad
 
 lpad:
   %0 = landingpad { i8*, i32 }
           catch i8* null
   br label %end
 
 end:
   ret void
 }
 
-; CHECK-LABEL: .Lhas_i32_arg_bitcast:
-; CHECK-NEXT: .functype .Lhas_i32_arg_bitcast (i32, i32) -> ()
-; CHECK-NEXT: call        has_i32_arg, $1{{$}}
-; CHECK-NEXT: end_function
-
-; CHECK-LABEL: .Lhas_i32_arg_bitcast.1:
-; CHECK-NEXT: .functype .Lhas_i32_arg_bitcast.1 (i32, i32) -> ()
-; CHECK-NEXT: call        has_i32_arg, $0{{$}}
-; CHECK-NEXT: end_function
-
-; CHECK-LABEL: .Lhas_i32_arg_bitcast.2:
-; CHECK:      call        has_i32_arg, $0{{$}}
-; CHECK-NEXT: end_function
+; TYPED-LABEL: .Lhas_i32_arg_bitcast:
+; TYPED-NEXT: .functype .Lhas_i32_arg_bitcast (i32, i32) -> ()
+; TYPED-NEXT: call        has_i32_arg, $1{{$}}
+; TYPED-NEXT: end_function
+
+; TYPED-LABEL: .Lhas_i32_arg_bitcast.1:
+; TYPED-NEXT: .functype .Lhas_i32_arg_bitcast.1 (i32, i32) -> ()
+; TYPED-NEXT: call        has_i32_arg, $0{{$}}
+; TYPED-NEXT: end_function
+
+; TYPED-LABEL: .Lhas_i32_arg_bitcast.2:
+; TYPED-NEXT: .functype .Lhas_i32_arg_bitcast.2 () -> ()
+; TYPED-NEXT: call        has_i32_arg, $0{{$}}
+; TYPED-NEXT: end_function
+
+; OPAQUE-LABEL: .Lhas_i32_arg_bitcast:
+; OPAQUE-NEXT: .functype  .Lhas_i32_arg_bitcast () -> ()
+; OPAQUE-NEXT: call        has_i32_arg, $0{{$}}
+; OPAQUE-NEXT: end_function
+
+; OPAQUE-LABEL: .Lhas_i32_arg_bitcast.1:
+; OPAQUE-NEXT: .functype .Lhas_i32_arg_bitcast.1 (i32, i32) -> ()
+; OPAQUE-NEXT: call        has_i32_arg, $1{{$}}
+; OPAQUE-NEXT: end_function
+
+; OPAQUE-LABEL: .Lhas_i32_arg_bitcast.2:
+; OPAQUE-NEXT: .functype .Lhas_i32_arg_bitcast.2 (i32, i32) -> ()
+; OPAQUE-NEXT: call        has_i32_arg, $0{{$}}
+; OPAQUE-NEXT: end_function
 
 ; CHECK-LABEL: .Lhas_i32_ret_bitcast:
 ; CHECK:      call        $drop=, has_i32_ret{{$}}
 ; CHECK-NEXT: end_function
 
 ; CHECK-LABEL: .Lvararg_bitcast:
 ; CHECK: call        vararg, $1{{$}}
 ; CHECK: end_function

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

 ; RUN: llc < %s -asm-verbose=false | FileCheck %s
+; RUN: llc < %s -asm-verbose=false -force-opaque-pointers | FileCheck %s
 
 ; Test main functions with alternate signatures.
 
 target triple = "wasm32-unknown-unknown"
 
 declare i32 @main()
 
 define i32 @foo() {
   %t = call i32 @main()
   ret i32 %t
 }
 
 ; CHECK-LABEL: foo:
 ; CHECK-NEXT:    .functype foo () -> (i32)
 ; CHECK-NEXT:    call __original_main
 ; CHECK-NEXT:    end_function