[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;
 };
 
 } // end namespace llvm
 
 #endif

llvm/lib/Target/WebAssembly/WebAssemblyTargetTransformInfo.cpp

unsigned WebAssemblyTTIImpl::getVectorInstrCost(unsigned Opcode, Type *Val,
   unsigned Cost = BasicTTIImplBase::getVectorInstrCost(Opcode, Val, Index);
 
   // SIMD128's insert/extract currently only take constant indices.
   if (Index == -1u)
     return Cost + 25 * TargetTransformInfo::TCC_Expensive;
 
   return Cost;
 }
+
+bool WebAssemblyTTIImpl::areInlineCompatible(const Function *Caller,
+                                             const Function *Callee) const {
+  // Allow inlining only when the Callee has a subset of the Caller's
+  // features. In principle, we should be able to inline regardless of any
+  // features because WebAssembly supports features at module granularity, not
+  // function granularity, but without this restriction it would be possible for
+  // a module to "forget" about features if all the functions that used them
+  // were inlined.
+  const TargetMachine &TM = getTLI()->getTargetMachine();
+
+  const FeatureBitset &CallerBits =
+      TM.getSubtargetImpl(*Caller)->getFeatureBits();
+  const FeatureBitset &CalleeBits =
+      TM.getSubtargetImpl(*Callee)->getFeatureBits();
+
+  return (CallerBits & CalleeBits) == CalleeBits;
+}

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 functions can be inlined into callers only when
+; they have a subset of the caller's features.
+
+target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
+target triple = "wasm32-unknown-unknown"
+
+declare void @foo()
+
+define internal void @uses_simd() #0 {
+; CHECK-LABEL: @uses_simd(
+; CHECK-NEXT:    tail call void @foo()
+; CHECK-NEXT:    ret void
+;
+  tail call void @foo()
+  ret void
+}
+
+define void @many_features() #1 {
+; CHECK-LABEL: @many_features(
+; CHECK-NEXT:    tail call void @foo()
+; CHECK-NEXT:    ret void
+;
+  tail call fastcc void @uses_simd()
+  ret void
+}
+
+define void @few_features() #2 {
+; CHECK-LABEL: @few_features(
+; CHECK-NEXT:    tail call fastcc void @uses_simd()
+; CHECK-NEXT:    ret void
+;
+  tail call fastcc void @uses_simd()
+  ret void
+}
+
+attributes #0 = { "target-cpu"="mvp" "target-features"="+simd128"}
+attributes #1 = { "target-cpu"="bleeding-edge" "target-features"="+simd128" }
+attributes #2 = { "target-cpu"="mvp" "target-features"="+multivalue" }