[WebAssembly] Allow inlining functions with different features

[tlively]

Summary

Because WebAssembly modules have to validate, having different feature
sets in different functions is not useful. As such, there is no reason
to prevent any function from being inlined into any other function due
to their enabled target features. This patch relaxes the inlining
restrictions for WebAssembly accordingly.

Frontends need to be careful, however, to set up their TargetMachines
with all the necessary features so that a feature will not be
"forgotten" because all the functions that used it were inlined and
removed from the module.

Requested in PR46812.

[tlively]

@alexcrichton As someone who works on an LLVM frontend, do you think the possibility that a feature can be "forgotten" would be worth introducing a restriction that functions can only be inlined into other functions that have a superset of their features?

[alexcrichton]

Ah that's actually a good point. I think in that case it may be best to only inline into functions which have a superset of features. A pretty common expected use is where you enable simd128 in some functions and then call a bunch of intrinsics. The standard library itself won't be compiled with simd128 by default, so that'll be the only way to use simd128.

We could recognize that if any feature is used anywhere in a module we enable it for the module, but that would behave pretty wasm-specific compared to other targets, so it's probably best to do a subset check?

[aheejin]

+1 on only inlining into functions which have a superset of features; it looks safer that way.

[tlively]

Sounds good, I'll make that change. @alexcrichton, would it be useful to have a separate LLVM pass that applies all features used anywhere in a module to each function in that module? Frontends could run that pass early when compiling to WebAssembly to remove the inlining restrictions.

[aheejin]

Where are you planning to run that pass? Inlining runs within opt, before we can have any chances to run our own passes. You mean clang by 'frontend'? Can we run our own pass in clang?

[tlively]

In D85494#2214339, @aheejin wrote:

Where are you planning to run that pass? Inlining runs within opt, before we can have any chances to run our own passes. You mean clang by 'frontend'? Can we run our own pass in clang?

Yeah, I was just looking at whether targets can run custom passes in clang. I didn't see any precedent for that, but it wouldn't be impossible.

[aheejin]

Not sure if it's worth it if there's no precedents. If people use our toolchain (clang or emscripten or rust or whatever) that wouldn't happen in the first place. Maybe it'd be sufficient to print some warning messages in CoalesceFeaturesAndStripAtomics if features don't match..?

[alexcrichton]

Yeah adding a pass to propagate the superset of all functions features to all functions sounds reasonable to me, it would be easy enough for that to be added to the pass manager on a wasm-specific basis.

[tlively]

• Require feature subsets

llvm/lib/Target/WebAssembly/WebAssemblyTargetTransformInfo.h

unsigned getArithmeticInstrCost(
       TTI::OperandValueKind Opd2Info = TTI::OK_AnyValue,
       TTI::OperandValueProperties Opd1PropInfo = TTI::OP_None,
       TTI::OperandValueProperties Opd2PropInfo = TTI::OP_None,
       ArrayRef<const Value *> Args = ArrayRef<const Value *>(),
       const Instruction *CxtI = nullptr);
   unsigned getVectorInstrCost(unsigned Opcode, Type *Val, unsigned Index);
 
   /// @}
+
+  bool areInlineCompatible(const Function *Caller,
+                           const Function *Callee) const {
+    // Since WebAssembly supports target features at the module level rather
+    // than the function level, the backend will take the union of all features
+    // and apply it to each function. That means that any function can be
+    // inlined into any other function regardless of the features
+    // used. Frontends should set features on the target machine properly to
+    // prevent features from being "forgotten" because all the functions that
+    // use them were inlined and removed from the module.
+    return true;
+  }
 };
 
 } // end namespace llvm
 
 #endif

llvm/test/Transforms/Inline/WebAssembly/inline-target-features.ll

+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -mtriple=wasm32-unknown-unknown -S -inline | FileCheck %s
+
+; Check that having different target attributes or cpus does not
+; inhibit inlining.
+
+target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
+target triple = "wasm32-unknown-unknown"
+
+define void @bar() #0 {
+; CHECK-LABEL: @bar(
+; CHECK-NEXT:    tail call void @foo()
+; CHECK-NEXT:    ret void
+;
+  tail call fastcc void @inline_me()
+  ret void
+}
+
+define internal void @inline_me() #1 {
+  tail call void @foo()
+  ret void
+}
+
+declare void @foo()
+
+attributes #0 = { "target-cpu"="mvp" "target-features"="+simd128" }
+attributes #1 = { "target-cpu"="bleeding-edge" "target-features"="+multivalue" }