[WebAssembly] Merge used feature sets, update atomics linkage policy

[tlively]

Summary

It does not currently make sense to use WebAssembly features in some functions
but not others, so this CL adds an IR pass that takes the union of all used
feature sets and applies it to each function in the module. This allows us to
prevent atomics from being lowered away if some function has opted in to using
them. When atomics is not enabled anywhere, we detect whether there exists any
atomic operations or thread local storage that would be stripped and disallow
linking with objects that contain atomics if and only if atomics or tls are
stripped. When atomics is enabled, mark it as used but do not require it of
other objects in the link. These changes allow libraries that do not use atomics
to be built once and linked into both single-threaded and multithreaded
binaries.

[aheejin]

llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp

320

Does this work in case we don't specify +matomics in the command line but only some of functions contains +matomics in their function attributes? In that case the TM's UsedFeatures set will be updated as we go, but we query the info before we look into any functions here.

(I know it's preexisting; I think I didn't review the previous CL that added this part. And I may not have the full context of your recent target feature section related CLs, in which case this may be a dumb question)

[tlively]

llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp

320

You're right that this is kind of subtle and that all the used features are not known at this point, but I think that the code behaves reasonably as written. If +atomics is not provided to the TargetMachine then all atomics and tls will be stripped. If some function later on enables atomics, then atomics will be added to the WebAssemblyTargetMachine's UsedFeatures, but since all atomics will have already been stripped, the output will still not contain any atomics. Since atomics were stripped, the target feature section correctly gets -atomics, even though they were "used".

However, I think a better design would be to add an IR pass to precompute all of the features used in the module. This would allow me to remove the mutable qualifier from UsedFeatures and would make the WebAssemblyTargetMachine safe to use for multiple modules. It would also allow us to strip atomics and tls only if no function in the module enables atomics, which is more consistent with how we treat features in the target features section.

[tlively]

• Calculate and use union of features used in module before starting instruction selection.

[tlively]

• Remove unnecessary test and small cleanup

[aheejin]

Mostly LGTM. Some nits and questions:

llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp

236

Then when is the atomics feature 'required' (WASM_FEATURE_PREFIX_REQUIRED) now? Shouldn't it be required when it is in the used the and WasmTM.getAtomicsStripped() is false?

llvm/lib/Target/WebAssembly/WebAssemblySubtarget.h

85

Where is this function used?

llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp

206

Does this mean the features string always ends with a comma?

211

clang-format?

214

• Do we need to do this to unify features between functions?
• Why do we remove target-cpu?

320

I agree that would be probably a better and safer design. By the way is there any case we use an instance of WebAssemblyTargetMachine for multiple modules?

llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.h

47–48

If this function is marked const, I guess it is supposed to return const FeatureBitSet &, considering now UsedFeatures is not a mutable member anymore?

48

We may not need that now, but probably this can be

void setAtomicsStripped(bool Value = true) { AtomicsStripped = V; }

to enable turning off the feature too.

48

The argument here can be const FeatureBitSet &

[tlively]

llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp

236

Currently WASM_FEATURE_PREFIX_REQUIRED is not used for anything. Originally we were using it as you described, but the problem is that if normal code with atomic ops has =atomics then it would not be linkable with thread-neutral libs like libpng. Using +atomics and -atomics instead of =atomics and -atomics is still safe with respect to stripped atomic ops, but also allows for thread-neutral objects.

We could entirely remove WASM_FEATURE_PREFIX_REQUIRED without breaking anything, but some people (Dan mostly) have expressed interest in a mode where all used features are strictly required. I'm not convinced that's useful, but I don't want to close the door on it either. Another potential use for WASM_FEATURE_PREFIX_REQUIRED would be enforcing ABI compatibility.

llvm/lib/Target/WebAssembly/WebAssemblySubtarget.h

85

Called by AtomicExpand::runOnFunction in AtomicExpandPass.cpp to determine whether any work needs to be done for that function. This lets us unconditionally run the AtomicExpandPass when we are setting up IR passes and defer the decision about whether or not to the expand atomics to pass run time.

llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp

206

Yes. This doesn't appear to change its semantics and it makes the code slightly simpler.

214

Yes, this is the part where we unify all the features. We remove target-cpu because any features it has enabled will be represented in the new target-features string, so it is redundant.

320

@dschuff just helped me investigate this, and it turns out WebAssemblyTargetMachine can be used to compile multiple modules. This means that we have to store the relevant information somewhere else. I believe attaching the target features as metadata to the Module itself is the best way to go. The Module is already supplied in the WebAssemblyAsmPrinter and obviously the Module is not reused between compilations. This will actually be a big code simplification!

[tlively]

• Switch to storing target feature information as Module metadata

[tlively]

llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp

211

Turns out my monorepo did not have my git hooks for this :( Should be fixed now.

[aheejin]

LGTM w/ nits. I like we can use module metadata this way.

llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp

203

Nit: Could be const FeatureBitSet &

228

Nit: Can we have {}s for outer loops too, because the inner loop has it?

265

Why is this an error? If module A uses a feature and B does not use it, doesn't the merged module use it? So shouldn't this be Module::ModFlagBehavior::Override too?

320

Just curious. In which case WebAssemblyTargetMachine is used to compile multiple modules?

[tlively]

• Address comments
• gracefully handle invalid metadata
• do not emit empty producers sections

llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp

265

The Error behavior only kicks in if two modules with different values for the same flag are merged. If a module with some flag is merged with another module without that flag, then there is no error. Since I do not emit metadata for unused features, this should not be a problem. Thinking about this more, I think the Override behavior for disallowed atomics should actually be Error to avoid a situation where a merged module uses atomics but also has atomics marked disallowed.

320

In llc there is an option to compile a module twice to make sure the results are the same. This option makes a clone of the input module and uses the same PassRunner (and therefore the same TargetMachine) to compile both of them. In that case this isn't actually a problem because the modules are the same, but it shows that some other tool (maybe a compile server) would be allowed to reuse the same TargetMachine for multiple modules.

llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.h

 //==- WebAssemblyMCTargetDesc.h - WebAssembly Target Descriptions -*- C++ -*-=//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file
 /// This file provides WebAssembly-specific target descriptions.
 ///
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_LIB_TARGET_WEBASSEMBLY_MCTARGETDESC_WEBASSEMBLYMCTARGETDESC_H
 #define LLVM_LIB_TARGET_WEBASSEMBLY_MCTARGETDESC_WEBASSEMBLYMCTARGETDESC_H
 
+#include "../WebAssemblySubtarget.h"
 #include "llvm/BinaryFormat/Wasm.h"
 #include "llvm/MC/MCInstrDesc.h"
 #include "llvm/Support/DataTypes.h"
 #include <memory>
 
 namespace llvm {
 
 class MCAsmBackend;

 #define GET_REGINFO_ENUM
 #include "WebAssemblyGenRegisterInfo.inc"
 
 // Defines symbolic names for the WebAssembly instructions.
 //
 #define GET_INSTRINFO_ENUM
 #include "WebAssemblyGenInstrInfo.inc"
 
-#define GET_SUBTARGETINFO_ENUM
-#include "WebAssemblyGenSubtargetInfo.inc"
-
 namespace llvm {
 namespace WebAssembly {
 
 /// Return the default p2align value for a load or store with the given opcode.
 inline unsigned GetDefaultP2Align(unsigned Opcode) {
   switch (Opcode) {
   case WebAssembly::LOAD8_S_I32:
   case WebAssembly::LOAD8_S_I32_S:

llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.h

public:
   }
 
   //===------------------------------------------------------------------===//
   // AsmPrinter Implementation.
   //===------------------------------------------------------------------===//
 
   void EmitEndOfAsmFile(Module &M) override;
   void EmitProducerInfo(Module &M);
-  void EmitTargetFeatures();
+  void EmitTargetFeatures(Module &M);
   void EmitJumpTableInfo() override;
   void EmitConstantPool() override;
   void EmitFunctionBodyStart() override;
   void EmitInstruction(const MachineInstr *MI) override;
   bool PrintAsmOperand(const MachineInstr *MI, unsigned OpNo,
                        unsigned AsmVariant, const char *ExtraCode,
                        raw_ostream &OS) override;
   bool PrintAsmMemoryOperand(const MachineInstr *MI, unsigned OpNo,

llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp

 #include "llvm/CodeGen/Analysis.h"
 #include "llvm/CodeGen/AsmPrinter.h"
 #include "llvm/CodeGen/MachineConstantPool.h"
 #include "llvm/CodeGen/MachineInstr.h"
 #include "llvm/CodeGen/MachineModuleInfoImpls.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/DebugInfoMetadata.h"
 #include "llvm/IR/GlobalVariable.h"
+#include "llvm/IR/Metadata.h"
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCSectionWasm.h"
 #include "llvm/MC/MCStreamer.h"
 #include "llvm/MC/MCSymbol.h"
 #include "llvm/MC/MCSymbolWasm.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/raw_ostream.h"
for (const Metadata *MD : Named->operands()) {
           OutContext.getWasmSection(SectionName, SectionKind::getMetadata());
       OutStreamer->SwitchSection(MySection);
       OutStreamer->EmitBytes(Contents->getString());
       OutStreamer->PopSection();
     }
   }
 
   EmitProducerInfo(M);
-  EmitTargetFeatures();
+  EmitTargetFeatures(M);
 }
 
 void WebAssemblyAsmPrinter::EmitProducerInfo(Module &M) {
   llvm::SmallVector<std::pair<std::string, std::string>, 4> Languages;
   if (const NamedMDNode *Debug = M.getNamedMetadata("llvm.dbg.cu")) {
     llvm::SmallSet<StringRef, 4> SeenLanguages;
     for (size_t I = 0, E = Debug->getNumOperands(); I < E; ++I) {
       const auto *CU = cast<DICompileUnit>(Debug->getOperand(I));
for (auto &Producers : {std::make_pair("language", &Languages),
         OutStreamer->EmitULEB128IntValue(Producer.second.size());
         OutStreamer->EmitBytes(Producer.second);
       }
     }
     OutStreamer->PopSection();
   }
 }
 
-void WebAssemblyAsmPrinter::EmitTargetFeatures() {
-  static const std::pair<unsigned, const char *> FeaturePairs[] = {
-      {WebAssembly::FeatureAtomics, "atomics"},
-      {WebAssembly::FeatureBulkMemory, "bulk-memory"},
-      {WebAssembly::FeatureExceptionHandling, "exception-handling"},
-      {WebAssembly::FeatureNontrappingFPToInt, "nontrapping-fptoint"},
-      {WebAssembly::FeatureSignExt, "sign-ext"},
-      {WebAssembly::FeatureSIMD128, "simd128"},
-  };
-
+void WebAssemblyAsmPrinter::EmitTargetFeatures(Module &M) {
   struct FeatureEntry {
     uint8_t Prefix;
     StringRef Name;
   };
 
-  FeatureBitset UsedFeatures =
-      static_cast<WebAssemblyTargetMachine &>(TM).getUsedFeatures();
-
-  // Calculate the features and linkage policies to emit
+  // Read target features and linkage policies from module metadata
   SmallVector<FeatureEntry, 4> EmittedFeatures;
-  for (auto &F : FeaturePairs) {
+  for (const SubtargetFeatureKV &KV : WebAssemblyFeatureKV) {
+    std::string MDKey = (StringRef("wasm-feature-") + KV.Key).str();
+    Metadata *Policy = M.getModuleFlag(MDKey);
+    if (Policy == nullptr)
+      continue;
+
     FeatureEntry Entry;
-    Entry.Name = F.second;
-    if (F.first == WebAssembly::FeatureAtomics) {
-      // "atomics" is special: code compiled without atomics may have had its
-      // atomics lowered to nonatomic operations. Such code would be dangerous
-      // to mix with proper atomics, so it is always Required or Disallowed.
-      Entry.Prefix = UsedFeatures[F.first]
-                         ? wasm::WASM_FEATURE_PREFIX_REQUIRED
-                         : wasm::WASM_FEATURE_PREFIX_DISALLOWED;
-      EmittedFeatures.push_back(Entry);
-    } else {
-      // Other features are marked Used or not mentioned
-      if (UsedFeatures[F.first]) {
-        Entry.Prefix = wasm::WASM_FEATURE_PREFIX_USED;
-        EmittedFeatures.push_back(Entry);
-      }
-    }
-  }
+    Entry.Prefix =
+        cast<ConstantInt>(cast<ConstantAsMetadata>(Policy)->getValue())
+            ->getZExtValue();
+    Entry.Name = KV.Key;
 
   // Emit features and linkage policies into the "target_features" section
   MCSectionWasm *FeaturesSection = OutContext.getWasmSection(
       ".custom_section.target_features", SectionKind::getMetadata());
   OutStreamer->PushSection();
   OutStreamer->SwitchSection(FeaturesSection);
 
   OutStreamer->EmitULEB128IntValue(EmittedFeatures.size());

llvm/lib/Target/WebAssembly/WebAssemblySubtarget.h

 
 #include "WebAssemblyFrameLowering.h"
 #include "WebAssemblyISelLowering.h"
 #include "WebAssemblyInstrInfo.h"
 #include "WebAssemblySelectionDAGInfo.h"
 #include "llvm/CodeGen/TargetSubtargetInfo.h"
 #include <string>
 
+#define GET_SUBTARGETINFO_ENUM
 #define GET_SUBTARGETINFO_HEADER
 #include "WebAssemblyGenSubtargetInfo.inc"
 
 namespace llvm {
 
+// Defined in WebAssemblyGenSubtargetInfo.inc.
+extern const SubtargetFeatureKV
+    WebAssemblyFeatureKV[WebAssembly::NumSubtargetFeatures];
+
 class WebAssemblySubtarget final : public WebAssemblyGenSubtargetInfo {
   enum SIMDEnum {
     NoSIMD,
     SIMD128,
     UnimplementedSIMD128,
   } SIMDLevel = NoSIMD;
 
   bool HasAtomics = false;
public:
   }
   const WebAssemblyInstrInfo *getInstrInfo() const override {
     return &InstrInfo;
   }
   const WebAssemblyRegisterInfo *getRegisterInfo() const override {
     return &getInstrInfo()->getRegisterInfo();
   }
   const Triple &getTargetTriple() const { return TargetTriple; }
+  bool enableAtomicExpand() const override;
   bool enableMachineScheduler() const override;
   bool useAA() const override;
 
   // Predicates used by WebAssemblyInstrInfo.td.
   bool hasAddr64() const { return TargetTriple.isArch64Bit(); }
   bool hasSIMD128() const { return SIMDLevel >= SIMD128; }
   bool hasUnimplementedSIMD128() const {
     return SIMDLevel >= UnimplementedSIMD128;

llvm/lib/Target/WebAssembly/WebAssemblySubtarget.cpp

WebAssemblySubtarget::WebAssemblySubtarget(const Triple &TT,
                                            const std::string &CPU,
                                            const std::string &FS,
                                            const TargetMachine &TM)
     : WebAssemblyGenSubtargetInfo(TT, CPU, FS), CPUString(CPU),
       TargetTriple(TT), FrameLowering(),
       InstrInfo(initializeSubtargetDependencies(FS)), TSInfo(),
       TLInfo(TM, *this) {}
 
+bool WebAssemblySubtarget::enableAtomicExpand() const {
+  // If atomics are disabled, atomic ops are lowered instead of expanded
+  return hasAtomics();
+}
+
 bool WebAssemblySubtarget::enableMachineScheduler() const {
   // Disable the MachineScheduler for now. Even with ShouldTrackPressure set and
   // enableMachineSchedDefaultSched overridden, it appears to have an overall
   // negative effect for the kinds of register optimizations we're doing.
   return false;
 }
 
 bool WebAssemblySubtarget::useAA() const { return true; }

llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.h

 #include "WebAssemblySubtarget.h"
 #include "llvm/Target/TargetMachine.h"
 
 namespace llvm {
 
 class WebAssemblyTargetMachine final : public LLVMTargetMachine {
   std::unique_ptr<TargetLoweringObjectFile> TLOF;
   mutable StringMap<std::unique_ptr<WebAssemblySubtarget>> SubtargetMap;
-  mutable FeatureBitset UsedFeatures;
 
 public:
   WebAssemblyTargetMachine(const Target &T, const Triple &TT, StringRef CPU,
                            StringRef FS, const TargetOptions &Options,
                            Optional<Reloc::Model> RM,
                            Optional<CodeModel::Model> CM, CodeGenOpt::Level OL,
                            bool JIT);
 
   ~WebAssemblyTargetMachine() override;
 
   const WebAssemblySubtarget *getSubtargetImpl(std::string CPU,
                                                std::string FS) const;
   const WebAssemblySubtarget *
   getSubtargetImpl(const Function &F) const override;
 
   // Pass Pipeline Configuration
   TargetPassConfig *createPassConfig(PassManagerBase &PM) override;
 
   TargetLoweringObjectFile *getObjFileLowering() const override {
     return TLOF.get();
   }
 
-  FeatureBitset getUsedFeatures() const { return UsedFeatures; }
-
   TargetTransformInfo getTargetTransformInfo(const Function &F) override;
 
   bool usesPhysRegsForPEI() const override { return false; }
 
   yaml::MachineFunctionInfo *createDefaultFuncInfoYAML() const override;
   yaml::MachineFunctionInfo *
   convertFuncInfoToYAML(const MachineFunction &MF) const override;
   bool parseMachineFunctionInfo(const yaml::MachineFunctionInfo &,
                                 PerFunctionMIParsingState &PFS,
                                 SMDiagnostic &Error,
                                 SMRange &SourceRange) const override;
 };
 
 } // end namespace llvm
 
 #endif

llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp

 #include "llvm/CodeGen/MachineFunctionPass.h"
 #include "llvm/CodeGen/Passes.h"
 #include "llvm/CodeGen/RegAllocRegistry.h"
 #include "llvm/CodeGen/TargetPassConfig.h"
 #include "llvm/IR/Function.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Target/TargetOptions.h"
 #include "llvm/Transforms/Scalar.h"
+#include "llvm/Transforms/Scalar/LowerAtomic.h"
 #include "llvm/Transforms/Utils.h"
 using namespace llvm;
 
 #define DEBUG_TYPE "wasm"
 
 // Emscripten's asm.js-style exception handling
 static cl::opt<bool> EnableEmException(
     "enable-emscripten-cxx-exceptions",
WebAssemblyTargetMachine::WebAssemblyTargetMachine(
   // WebAssembly treats each function as an independent unit. Force
   // -ffunction-sections, effectively, so that we can emit them independently.
   this->Options.FunctionSections = true;
   this->Options.DataSections = true;
   this->Options.UniqueSectionNames = true;
 
   initAsmInfo();
 
-  // Create a subtarget using the unmodified target machine features to
-  // initialize the used feature set with explicitly enabled features.
-  getSubtargetImpl(getTargetCPU(), getTargetFeatureString());
-
   // Note that we don't use setRequiresStructuredCFG(true). It disables
   // optimizations than we're ok with, and want, such as critical edge
   // splitting and tail merging.
 }
 
 WebAssemblyTargetMachine::~WebAssemblyTargetMachine() = default; // anchor.
 
 const WebAssemblySubtarget *
 WebAssemblyTargetMachine::getSubtargetImpl(std::string CPU,
                                            std::string FS) const {
   auto &I = SubtargetMap[CPU + FS];
   if (!I) {
     I = llvm::make_unique<WebAssemblySubtarget>(TargetTriple, CPU, FS, *this);
-    UsedFeatures |= I->getFeatureBits();
   }
   return I.get();
 }
 
 const WebAssemblySubtarget *
 WebAssemblyTargetMachine::getSubtargetImpl(const Function &F) const {
   Attribute CPUAttr = F.getFnAttribute("target-cpu");
   Attribute FSAttr = F.getFnAttribute("target-features");
WebAssemblyTargetMachine::getSubtargetImpl(const Function &F) const {
   // creation will depend on the TM and the code generation flags on the
   // function that reside in TargetOptions.
   resetTargetOptions(F);
 
   return getSubtargetImpl(CPU, FS);
 }
 
 namespace {
-class StripThreadLocal final : public ModulePass {
-  // The default thread model for wasm is single, where thread-local variables
-  // are identical to regular globals and should be treated the same. So this
-  // pass just converts all GlobalVariables to NotThreadLocal
+
+class CoalesceFeaturesAndStripAtomics final : public ModulePass {
+  // Take the union of all features used in the module and use it for each
+  // function individually, since having multiple feature sets in one module
+  // currently does not make sense for WebAssembly. If atomics are not enabled,
+  // also strip atomic operations and thread local storage.
   static char ID;
+  WebAssemblyTargetMachine *WasmTM;
 
 public:
-  StripThreadLocal() : ModulePass(ID) {}
+  CoalesceFeaturesAndStripAtomics(WebAssemblyTargetMachine *WasmTM)
+      : ModulePass(ID), WasmTM(WasmTM) {}
+
   bool runOnModule(Module &M) override {
-    for (auto &GV : M.globals())
-      GV.setThreadLocalMode(GlobalValue::ThreadLocalMode::NotThreadLocal);
+    FeatureBitset Features = coalesceFeatures(M);
+
+    std::string FeatureStr = getFeatureString(Features);
+    for (auto &F : M)
+      replaceFeatures(F, FeatureStr);
+
+    bool Stripped = false;
+    if (!Features[WebAssembly::FeatureAtomics]) {
+      Stripped |= stripAtomics(M);
+      Stripped |= stripThreadLocals(M);
+    }
+
+    recordFeatures(M, Features, Stripped);
+
+    // Conservatively assume we have made some change
     return true;
   }
+
+private:
+  FeatureBitset coalesceFeatures(const Module &M) {
+    FeatureBitset Features =
+        WasmTM
+            ->getSubtargetImpl(WasmTM->getTargetCPU(),
+                               WasmTM->getTargetFeatureString())
+            ->getFeatureBits();
+    for (auto &F : M)
+      Features |= WasmTM->getSubtargetImpl(F)->getFeatureBits();
+    return Features;
+  }
+
+  std::string getFeatureString(FeatureBitset Features) {
+    std::string Ret;
+    for (const SubtargetFeatureKV &KV : WebAssemblyFeatureKV) {
+      if (Features[KV.Value])
+        Ret += (StringRef("+") + KV.Key + ",").str();
+    }
+    return Ret;
+  }
+
+  void replaceFeatures(Function &F, const std::string &Features) {
+    F.removeFnAttr("target-features");
+    F.removeFnAttr("target-cpu");
+    F.addFnAttr("target-features", Features);
+  }
+
+  bool stripAtomics(Module &M) {
+    // Detect whether any atomics will be lowered, since there is no way to tell
+    // whether the LowerAtomic pass lowers e.g. stores.
+    bool Stripped = false;
+    for (auto &F : M)
+      for (auto &B : F)
+        for (auto &I : B)
+          if (I.isAtomic()) {
+            Stripped = true;
+            goto done;
+          }
+  done:
+    if (!Stripped)
+      return false;
+
+    LowerAtomicPass Lowerer;
+    FunctionAnalysisManager FAM;
+    for (auto &F : M)
+      Lowerer.run(F, FAM);
+
+    return true;
+  }
+
+  bool stripThreadLocals(Module &M) {
+    bool Stripped = false;
+    for (auto &GV : M.globals()) {
+      if (GV.getThreadLocalMode() !=
+          GlobalValue::ThreadLocalMode::NotThreadLocal) {
+        Stripped = true;
+        GV.setThreadLocalMode(GlobalValue::ThreadLocalMode::NotThreadLocal);
+      }
+    }
+    return Stripped;
+  }
+
+  void recordFeatures(Module &M, const FeatureBitset &Features, bool Stripped) {
+    for (const SubtargetFeatureKV &KV : WebAssemblyFeatureKV) {
+      if (KV.Value == WebAssembly::FeatureAtomics && Stripped) {
+        // "atomics" is special: code compiled without atomics may have had its
+        // atomics lowered to nonatomic operations. In that case, atomics is
+        // disallowed to prevent unsafe linking with atomics-enabled objects.
+        assert(!Features[WebAssembly::FeatureAtomics]);
+        M.addModuleFlag(Module::ModFlagBehavior::Override,
+                        (StringRef("wasm-feature-") + KV.Key).str(),
+                        wasm::WASM_FEATURE_PREFIX_DISALLOWED);
+      } else if (Features[KV.Value]) {
+        // Otherwise features are marked Used or not mentioned
+        M.addModuleFlag(Module::ModFlagBehavior::Error,
+                        (StringRef("wasm-feature-") + KV.Key).str(),
+                        wasm::WASM_FEATURE_PREFIX_USED);
+      }
+    }
+  }
 };
-char StripThreadLocal::ID = 0;
+char CoalesceFeaturesAndStripAtomics::ID = 0;
 
 /// WebAssembly Code Generator Pass Configuration Options.
 class WebAssemblyPassConfig final : public TargetPassConfig {
 public:
   WebAssemblyPassConfig(WebAssemblyTargetMachine &TM, PassManagerBase &PM)
       : TargetPassConfig(TM, PM) {}
 
   WebAssemblyTargetMachine &getWebAssemblyTargetMachine() const {

 }
 
 //===----------------------------------------------------------------------===//
 // The following functions are called from lib/CodeGen/Passes.cpp to modify
 // the CodeGen pass sequence.
 //===----------------------------------------------------------------------===//
 
 void WebAssemblyPassConfig::addIRPasses() {
-  if (static_cast<WebAssemblyTargetMachine *>(TM)
-          ->getUsedFeatures()[WebAssembly::FeatureAtomics]) {
-    // Expand some atomic operations. WebAssemblyTargetLowering has hooks which
-    // control specifically what gets lowered.
-    addPass(createAtomicExpandPass());
-  } else {
-    // If atomics are not enabled, they get lowered to non-atomics.
-    addPass(createLowerAtomicPass());
-    addPass(new StripThreadLocal());
-  }
+  // Runs LowerAtomicPass if necessary
+  addPass(new CoalesceFeaturesAndStripAtomics(&getWebAssemblyTargetMachine()));
+
+  // This is a no-op if atomics are not used in the module
 
   // Add signatures to prototype-less function declarations
   addPass(createWebAssemblyAddMissingPrototypes());
 
   // Lower .llvm.global_dtors into .llvm_global_ctors with __cxa_atexit calls.
   addPass(createWebAssemblyLowerGlobalDtors());
 
   // Fix function bitcasts, as WebAssembly requires caller and callee signatures

llvm/test/CodeGen/WebAssembly/target-features-tls.ll

+; RUN: llc < %s -mattr=-atomics | FileCheck %s --check-prefixes CHECK,NO-ATOMICS
+; RUN: llc < %s -mattr=+atomics | FileCheck %s --check-prefixes CHECK,ATOMICS
+
+; Test that the target features section contains -atomics or +atomics
+; for modules that have thread local storage in their source.
+
+target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
+target triple = "wasm32-unknown-unknown"
+
+@foo = internal thread_local global i32 0
+
+; CHECK-LABEL: .custom_section.target_features,"",@
+
+; -atomics
+; NO-ATOMICS-NEXT: .int8 1
+; NO-ATOMICS-NEXT: .int8 45
+; NO-ATOMICS-NEXT: .int8 7
+; NO-ATOMICS-NEXT: .ascii "atomics"
+; NO-ATOMICS-NEXT: .bss.foo,"",@
+
+; +atomics
+; ATOMICS-NEXT: .int8 1
+; ATOMICS-NEXT: .int8 43
+; ATOMICS-NEXT: .int8 7
+; ATOMICS-NEXT: .ascii "atomics"
+; ATOMICS-NEXT: .tbss.foo,"",@

llvm/test/CodeGen/WebAssembly/target-features.ll

 ; RUN: llc < %s | FileCheck %s --check-prefixes CHECK,ATTRS
 ; RUN: llc < %s -mattr=+simd128 | FileCheck %s --check-prefixes CHECK,SIMD128
-; RUN; llc < %s -mattr=+atomics | FileCheck %s --check-prefixes CHECK,ATOMICS
 ; RUN: llc < %s -mcpu=bleeding-edge | FileCheck %s --check-prefixes CHECK,BLEEDING-EDGE
 
 ; Test that codegen emits target features from the command line or
-; function attributes correctly.
+; function attributes correctly and that features are enabled for the
+; entire module if they are enabled for any function in the module.
 
 target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
 target triple = "wasm32-unknown-unknown"
 
-define void @foo() #0 {
+define void @foo(i32* %p1) #0 {
+  %a = atomicrmw min i32* undef, i32 42 seq_cst
+  %v = fptoui float undef to i32
+  store i32 %v, i32* %p1
   ret void
 }
 
-define void @bar() #1 {
+define void @bar(i32* %p1) #1 {
+  %a = atomicrmw min i32* undef, i32 42 seq_cst
+  %v = fptoui float undef to i32
+  store i32 %v, i32* %p1
   ret void
 }
 
-attributes #0 = { "target-features"="+sign-ext" }
+attributes #0 = { "target-features"="+atomics" }
 attributes #1 = { "target-features"="+nontrapping-fptoint" }
 
+
+; CHECK-LABEL: foo:
+
+; Expanded atomicrmw min
+; ATTRS:       loop
+; ATTRS:       i32.atomic.rmw.cmpxchg
+; SIMD128-NOT: i32.atomic.rmw.cmpxchg
+; ATTRS:       end_loop
+
+; nontrapping fptoint
+; ATTRS:       i32.trunc_sat_f32_u
+; SIMD128-NOT: i32.trunc_sat_f32_u
+; ATTRS:       i32.store
+
+; `bar` should be the same as `foo`
+; CHECK-LABEL: bar:
+
+; Expanded atomicrmw min
+; ATTRS:       loop
+; ATTRS:       i32.atomic.rmw.cmpxchg
+; SIMD128-NOT: i32.atomic.rmw.cmpxchg
+; ATTRS:       end_loop
+
+; nontrapping fptoint
+; ATTRS:       i32.trunc_sat_f32_u
+; SIMD128-NOT: i32.trunc_sat_f32_u
+; ATTRS:       i32.store
+
 ; CHECK-LABEL: .custom_section.target_features,"",@
 
-; -atomics, +sign_ext
-; ATTRS-NEXT: .int8 3
-; ATTRS-NEXT: .int8 45
+; +atomics, +nontrapping-fptoint
+; ATTRS-NEXT: .int8 2
+; ATTRS-NEXT: .int8 43
 ; ATTRS-NEXT: .int8 7
 ; ATTRS-NEXT: .ascii "atomics"
 ; ATTRS-NEXT: .int8 43
 ; ATTRS-NEXT: .int8 19
 ; ATTRS-NEXT: .ascii "nontrapping-fptoint"
-; ATTRS-NEXT: .int8 43
-; ATTRS-NEXT:  int8 8
-; ATTRS-NEXT: .ascii "sign-ext"
 
 ; -atomics, +simd128
 ; SIMD128-NEXT: .int8 2
 ; SIMD128-NEXT: .int8 45
 ; SIMD128-NEXT: .int8 7
 ; SIMD128-NEXT: .ascii "atomics"
 ; SIMD128-NEXT: .int8 43
 ; SIMD128-NEXT: .int8 7
 ; SIMD128-NEXT: .ascii "simd128"
 
-; =atomics
-; ATOMICS-NEXT: .int8 1
-; ATOMICS-NEXT: .int8 61
-; ATOMICS-NEXT: .int8 7
-; ATOMICS-NEXT: .ascii "atomics"
-
-; =atomics, +nontrapping-fptoint, +sign-ext, +simd128
+; +atomics, +nontrapping-fptoint, +sign-ext, +simd128
 ; BLEEDING-EDGE-NEXT: .int8   4
-; BLEEDING-EDGE-NEXT: .int8   61
+; BLEEDING-EDGE-NEXT: .int8   43
 ; BLEEDING-EDGE-NEXT: .int8   7
 ; BLEEDING-EDGE-NEXT: .ascii  "atomics"
 ; BLEEDING-EDGE-NEXT: .int8   43
 ; BLEEDING-EDGE-NEXT: .int8   19
 ; BLEEDING-EDGE-NEXT: .ascii  "nontrapping-fptoint"
 ; BLEEDING-EDGE-NEXT: .int8   43
 ; BLEEDING-EDGE-NEXT: .int8   8
 ; BLEEDING-EDGE-NEXT: .ascii  "sign-ext"
 ; BLEEDING-EDGE-NEXT: .int8   43
 ; BLEEDING-EDGE-NEXT: .int8   7
 ; BLEEDING-EDGE-NEXT: .ascii  "simd128"
 
 ; CHECK-NEXT: .text

llvm/test/MC/WebAssembly/array-fill.ll

 ; CHECK-NEXT:         Size:            2
 ; CHECK-NEXT:     SegmentInfo:
 ; CHECK-NEXT:       - Index:           0
 ; CHECK-NEXT:         Name:            .data
 ; CHECK-NEXT:         Alignment:       0
 ; CHECK-NEXT:         Flags:           [ ]
 ; CHECK-NEXT:   - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            target_features
-; CHECK-NEXT:     Features:
-; CHECK-NEXT:       - Prefix:          DISALLOWED
-; CHECK-NEXT:         Name:            atomics
+; CHECK-NEXT:     Features:        []
 ; CHECK-NEXT: ...

llvm/test/MC/WebAssembly/assembler-binary.ll

 ; CHECK-NEXT:         Function:        1
 ; CHECK-NEXT:       - Index:           1
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Name:            bar
 ; CHECK-NEXT:         Flags:           [ UNDEFINED ]
 ; CHECK-NEXT:         Function:        0
 ; CHECK-NEXT:   - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            target_features
-; CHECK-NEXT:     Features:
-; CHECK-NEXT:       - Prefix:          DISALLOWED
-; CHECK-NEXT:         Name:            atomics
+; CHECK-NEXT:     Features:        []
 ; CHECK-NEXT: ...

llvm/test/MC/WebAssembly/bss.ll

 ; CHECK-NEXT:         Alignment:       0
 ; CHECK-NEXT:         Flags:           [ ]
 ; CHECK-NEXT:       - Index:           3
 ; CHECK-NEXT:         Name:            .bss.bar
 ; CHECK-NEXT:         Alignment:       0
 ; CHECK-NEXT:         Flags:           [ ]
 ; CHECK-NEXT:   - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            target_features
-; CHECK-NEXT:     Features:
-; CHECK-NEXT:       - Prefix:          DISALLOWED
-; CHECK-NEXT:         Name:            atomics
+; CHECK-NEXT:     Features:        []
 ; CHECK-NEXT: ...

llvm/test/MC/WebAssembly/comdat.ll

 ; CHECK-NEXT:        Name:            sharedComdat
 ; CHECK-NEXT:        Entries:
 ; CHECK-NEXT:          - Kind:            FUNCTION
 ; CHECK-NEXT:            Index:           3
 ; CHECK-NEXT:          - Kind:            DATA
 ; CHECK-NEXT:            Index:           0
 ; CHECK-NEXT:   - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            target_features
-; CHECK-NEXT:     Features:
-; CHECK-NEXT:       - Prefix:          DISALLOWED
-; CHECK-NEXT:         Name:            atomics
+; CHECK-NEXT:     Features:        []
 ; CHECK-NEXT: ...

llvm/test/MC/WebAssembly/debug-info.ll

 ; CHECK-NEXT:  Section {
 ; CHECK-NEXT:    Type: CUSTOM (0x0)
 ; CHECK-NEXT:    Size: 77
 ; CHECK-NEXT:    Offset: 1021
 ; CHECK-NEXT:    Name: producers
 ; CHECK-NEXT:  }
 ; CHECK-NEXT:  Section {
 ; CHECK-NEXT:    Type: CUSTOM (0x0)
-; CHECK-NEXT:    Size: 10
+; CHECK-NEXT:    Size: 1
 ; CHECK-NEXT:    Offset: 1114
 ; CHECK-NEXT:    Name: target_features
 ; CHECK-NEXT:  }
 ; CHECK-NEXT:]
 ; CHECK-NEXT:Relocations [
 ; CHECK-NEXT:  Section (6) DATA {
 ; CHECK-NEXT:    0x6 R_WASM_MEMORY_ADDR_I32 myextern 0
 ; CHECK-NEXT:    0xF R_WASM_TABLE_INDEX_I32 f2

llvm/test/MC/WebAssembly/explicit-sections.ll

 ; CHECK-NEXT:         Alignment:       3
 ; CHECK-NEXT:         Flags:           [ ]
 ; CHECK-NEXT:       - Index:           2
 ; CHECK-NEXT:         Name:            .sec2
 ; CHECK-NEXT:         Alignment:       3
 ; CHECK-NEXT:         Flags:           [ ]
 ; CHECK-NEXT:   - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            target_features
-; CHECK-NEXT:     Features:
-; CHECK-NEXT:       - Prefix:          DISALLOWED
-; CHECK-NEXT:         Name:            atomics
+; CHECK-NEXT:     Features:        []
 ; CHECK-NEXT: ...

llvm/test/MC/WebAssembly/global-ctor-dtor.ll

 ; CHECK-NEXT:       - Priority: 42
 ; CHECK-NEXT:         Symbol: 2
 ; CHECK-NEXT:       - Priority: 65535
 ; CHECK-NEXT:         Symbol: 10
 ; CHECK-NEXT:       - Priority: 65535
 ; CHECK-NEXT:         Symbol: 7
 ; CHECK-NEXT:   - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            target_features
-; CHECK-NEXT:     Features:
-; CHECK-NEXT:       - Prefix:          DISALLOWED
-; CHECK-NEXT:         Name:            atomics
+; CHECK-NEXT:     Features:        []
 ; CHECK-NEXT: ...

llvm/test/MC/WebAssembly/visibility.ll

 ; CHECK-NEXT:         Function:        0
 ; CHECK-NEXT:       - Index:           1
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Name:            hiddenVis
 ; CHECK-NEXT:         Flags:           [ VISIBILITY_HIDDEN ]
 ; CHECK-NEXT:         Function:        1
 ; CHECK-NEXT:   - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            target_features
-; CHECK-NEXT:     Features:
-; CHECK-NEXT:       - Prefix:          DISALLOWED
-; CHECK-NEXT:         Name:            atomics
+; CHECK-NEXT:     Features:        []
 ; CHECK-NEXT: ...

llvm/test/MC/WebAssembly/weak-alias.ll

 ; CHECK-NEXT:         Alignment:       3
 ; CHECK-NEXT:         Flags:           [ ]
 ; CHECK-NEXT:       - Index:           2
 ; CHECK-NEXT:         Name:            .data.alias_address
 ; CHECK-NEXT:         Alignment:       3
 ; CHECK-NEXT:         Flags:           [ ]
 ; CHECK-NEXT:   - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            target_features
-; CHECK-NEXT:     Features:
-; CHECK-NEXT:       - Prefix:          DISALLOWED
-; CHECK-NEXT:         Name:            atomics
+; CHECK-NEXT:     Features:        []
 ; CHECK-NEXT: ...
 
 ; CHECK-SYMS: SYMBOL TABLE:
 ; CHECK-SYMS-NEXT: 00000001 g     F CODE  .hidden foo
 ; CHECK-SYMS-NEXT: 00000006 g     F CODE  .hidden call_direct
 ; CHECK-SYMS-NEXT: 0000000f g     F CODE  .hidden call_alias
 ; CHECK-SYMS-NEXT: 00000001 gw    F CODE  .hidden foo_alias
 ; CHECK-SYMS-NEXT: 00000018 g     F CODE  .hidden call_direct_ptr
 ; CHECK-SYMS-NEXT: 00000008 g     O DATA  direct_address
 ; CHECK-SYMS-NEXT: 0000002b g     F CODE  .hidden call_alias_ptr
 ; CHECK-SYMS-NEXT: 00000010 g     O DATA  alias_address
 ; CHECK-SYMS-NEXT: 00000000 g     O DATA  bar
 ; CHECK-SYMS-NEXT: 00000000 gw    O DATA  .hidden bar_alias

llvm/test/MC/WebAssembly/weak.ll

 ; CHECK-NEXT:         Flags:           [ BINDING_WEAK, VISIBILITY_HIDDEN ]
 ; CHECK-NEXT:         Function:        0
 ; CHECK-NEXT:       - Index:           1
 ; CHECK-NEXT:         Kind:            DATA
 ; CHECK-NEXT:         Name:            weak_external_data
 ; CHECK-NEXT:         Flags:           [ BINDING_WEAK, UNDEFINED ]
 ; CHECK-NEXT:   - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            target_features
-; CHECK-NEXT:     Features:
-; CHECK-NEXT:       - Prefix:          DISALLOWED
-; CHECK-NEXT:         Name:            atomics
+; CHECK-NEXT:     Features:        []
 ; CHECK-NEXT: ...

llvm/utils/TableGen/SubtargetEmitter.cpp

void SubtargetEmitter::Enumeration(raw_ostream &OS,
   // Get all records of class and sort
   std::vector<Record*> DefList =
     Records.getAllDerivedDefinitions("SubtargetFeature");
   llvm::sort(DefList, LessRecord());
 
   unsigned N = DefList.size();
   if (N == 0)
     return;
-  if (N > MAX_SUBTARGET_FEATURES)
+  if (N + 1 > MAX_SUBTARGET_FEATURES)
     PrintFatalError("Too many subtarget features! Bump MAX_SUBTARGET_FEATURES.");
 
   OS << "namespace " << Target << " {\n";
 
   // Open enumeration.
   OS << "enum {\n";
 
   // For each record
   for (unsigned i = 0; i < N; ++i) {
     // Next record
     Record *Def = DefList[i];
 
     // Get and emit name
     OS << "  " << Def->getName() << " = " << i << ",\n";
 
     // Save the index for this feature.
     FeatureMap[Def] = i;
   }
 
+  OS << "  "
+     << "NumSubtargetFeatures = " << N << "\n";
+
   // Close enumeration and namespace
   OS << "};\n";
   OS << "} // end namespace " << Target << "\n";
 }
 
 static void printFeatureMask(raw_ostream &OS, RecVec &FeatureList,
                              const DenseMap<Record *, unsigned> &FeatureMap) {
   std::array<uint64_t, MAX_SUBTARGET_WORDS> Mask = {};