Changeset View
Standalone View
llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
| Show First 20 Lines • Show All 183 Lines • ▼ Show 20 Lines | bool runOnModule(Module &M) override { | ||||
| std::string FeatureStr = getFeatureString(Features); | std::string FeatureStr = getFeatureString(Features); | ||||
| for (auto &F : M) | for (auto &F : M) | ||||
| replaceFeatures(F, FeatureStr); | replaceFeatures(F, FeatureStr); | ||||
| bool Stripped = false; | bool Stripped = false; | ||||
| if (!Features[WebAssembly::FeatureAtomics]) { | if (!Features[WebAssembly::FeatureAtomics]) { | ||||
| Stripped |= stripAtomics(M); | Stripped |= stripAtomics(M); | ||||
| Stripped |= stripThreadLocals(M); | |||||
aheejin: Looks like this feature requires both bulk memory and mutable global. Shouldn't we strip thread… | |||||
Do we want to do that? I think having your thread locals vanish if you didn't pass -mbulk-memory is surprising. It might make more sense to strip thread locals if -pthread is not passed. quantum: Do we want to do that? I think having your thread locals vanish if you didn't pass `-mbulk… | |||||
I have a patch up to make -pthread imply -mbulk-memory, so this shouldn't be a problem from a UI perspective. I agree with @aheejin that we should strip TLS if the necessary features aren't present to preserver feature-correctness. All the errors for using TLS without bulk memory should also apply to mutable-globals, which I hadn't considered before. I will update my patch so -pthread implies -mmutable-globals as well. tlively: I have a patch up to make `-pthread` imply `-mbulk-memory`, so this shouldn't be a problem from… | |||||
Going to strip out TLS if bulk-memory is not enabled. quantum: Going to strip out TLS if `bulk-memory` is not enabled. | |||||
| } | } | ||||
I just realized that if we have atomics but not bulk memory and TLS is stripped, then we will be in the awkward situation of both using atomics and disallowing atomics because the module is not thread safe. I think the best solution would be to go back and forcibly strip both atomics and TLS if either of them would be stripped. tlively: I just realized that if we have atomics but not bulk memory and TLS is stripped, then we will… | |||||
Done. quantum: Done. | |||||
We can be more fine grained than this. We should strip atomics or TLS only if the relevant feature is missing or the other one is stripped. It is possible to have thread-locals and bulk memory but not have any atomics to strip. tlively: We can be more fine grained than this. We should strip atomics or TLS only if the relevant… | |||||
| recordFeatures(M, Features, Stripped); | recordFeatures(M, Features, Stripped); | ||||
| // Conservatively assume we have made some change | // Conservatively assume we have made some change | ||||
| return true; | return true; | ||||
| } | } | ||||
| private: | private: | ||||
| FeatureBitset coalesceFeatures(const Module &M) { | FeatureBitset coalesceFeatures(const Module &M) { | ||||
| ▲ Show 20 Lines • Show All 44 Lines • ▼ Show 20 Lines | done: | ||||
| LowerAtomicPass Lowerer; | LowerAtomicPass Lowerer; | ||||
| FunctionAnalysisManager FAM; | FunctionAnalysisManager FAM; | ||||
| for (auto &F : M) | for (auto &F : M) | ||||
| Lowerer.run(F, FAM); | Lowerer.run(F, FAM); | ||||
| return true; | 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) { | void recordFeatures(Module &M, const FeatureBitset &Features, bool Stripped) { | ||||
| for (const SubtargetFeatureKV &KV : WebAssemblyFeatureKV) { | for (const SubtargetFeatureKV &KV : WebAssemblyFeatureKV) { | ||||
| std::string MDKey = (StringRef("wasm-feature-") + KV.Key).str(); | std::string MDKey = (StringRef("wasm-feature-") + KV.Key).str(); | ||||
| if (KV.Value == WebAssembly::FeatureAtomics && Stripped) { | if (KV.Value == WebAssembly::FeatureAtomics && Stripped) { | ||||
| // "atomics" is special: code compiled without atomics may have had its | // "atomics" is special: code compiled without atomics may have had its | ||||
| // atomics lowered to nonatomic operations. In that case, atomics is | // atomics lowered to nonatomic operations. In that case, atomics is | ||||
| // disallowed to prevent unsafe linking with atomics-enabled objects. | // disallowed to prevent unsafe linking with atomics-enabled objects. | ||||
| assert(!Features[WebAssembly::FeatureAtomics]); | assert(!Features[WebAssembly::FeatureAtomics]); | ||||
| ▲ Show 20 Lines • Show All 224 Lines • Show Last 20 Lines | |||||
Looks like this feature requires both bulk memory and mutable global. Shouldn't we strip thread locals if either is not enabled?