[WebAssembly] avoid to enable explicit disabled feature

[HerrCai0907]

In CoalesceFeaturesAndStripAtomics, feature string is converted to FeatureBitset and back to feature string. It will lose information about explicit diasbled features.

[llvmbot]

@llvm/pr-subscribers-backend-webassembly

Author: Congcong Cai (HerrCai0907)

Changes

In CoalesceFeaturesAndStripAtomics, feature string is converted to FeatureBitset and back to feature string. It will lose information about explicit diasbled features.

---

Full diff: https://github.com/llvm/llvm-project/pull/80094.diff

2 Files Affected:

• (modified) llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp (+8-2)
• (added) llvm/test/CodeGen/WebAssembly/disable-feature.ll (+22)

diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
index 2db1b6493cc47..0aadc6aca228d 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
@@ -200,7 +200,8 @@ class CoalesceFeaturesAndStripAtomics final : public ModulePass {
   bool runOnModule(Module &M) override {
     FeatureBitset Features = coalesceFeatures(M);
 
-    std::string FeatureStr = getFeatureString(Features);
+    std::string FeatureStr =
+        getFeatureString(Features, WasmTM->getTargetFeatureString());
     WasmTM->setTargetFeatureString(FeatureStr);
     for (auto &F : M)
       replaceFeatures(F, FeatureStr);
@@ -238,12 +239,17 @@ class CoalesceFeaturesAndStripAtomics final : public ModulePass {
     return Features;
   }
 
-  std::string getFeatureString(const FeatureBitset &Features) {
+  static std::string getFeatureString(const FeatureBitset &Features,
+                                      StringRef TargetFS) {
     std::string Ret;
     for (const SubtargetFeatureKV &KV : WebAssemblyFeatureKV) {
       if (Features[KV.Value])
         Ret += (StringRef("+") + KV.Key + ",").str();
     }
+    SubtargetFeatures TF{TargetFS};
+    for (std::string const &F : TF.getFeatures())
+      if (!SubtargetFeatures::isEnabled(F))
+        Ret += F;
     return Ret;
   }
 
diff --git a/llvm/test/CodeGen/WebAssembly/disable-feature.ll b/llvm/test/CodeGen/WebAssembly/disable-feature.ll
new file mode 100644
index 0000000000000..a9b07e2aaed5d
--- /dev/null
+++ b/llvm/test/CodeGen/WebAssembly/disable-feature.ll
@@ -0,0 +1,22 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mattr=-sign-ext | FileCheck %s
+
+target triple = "wasm32-unknown-unknown"
+
+define i8 @ashr(i8 %v, i8 %x) {
+; CHECK-LABEL: ashr:
+; CHECK:         .functype ashr (i32, i32) -> (i32)
+; CHECK-NEXT:  # %bb.0:
+; CHECK-NEXT:    local.get 0
+; CHECK-NEXT:    i32.const 24
+; CHECK-NEXT:    i32.shl
+; CHECK-NEXT:    i32.const 24
+; CHECK-NEXT:    i32.shr_s
+; CHECK-NEXT:    local.get 1
+; CHECK-NEXT:    i32.const 255
+; CHECK-NEXT:    i32.and
+; CHECK-NEXT:    i32.shr_s
+; CHECK-NEXT:    # fallthrough-return
+  %a = ashr i8 %v, %x
+  ret i8 %a
+}

[tlively]

What should happen if a feature is explicitly disabled via globally, but enabled on one of the functions in the module? IIUC, this current patch will have the string to enable it followed by the string to disable it. Which one wins? I think it would be good to have the feature enabled in that case.

[HerrCai0907]

What should happen if a feature is explicitly disabled via globally, but enabled on one of the functions in the module?

How could this case happen? If explicitly disabling a feature, it should not be enabled on function.

[HerrCai0907]

I think it would be good to have the feature enabled in that case.

I think it should throw diagnose or assert it won't happen. Otherwise the user will find they disable some feature but code still has this feature.

[HerrCai0907]

I have do some testing, for

attributes #0 = { noinline nounwind optnone "target-features"="+bulk-memory," }
declare void @llvm.memset.p0.i32(ptr, i8, i32, i1)
define void @memset_alloca(i8 %val) {
  %a = alloca [100 x i8]
  call void @llvm.memset.p0.i32(ptr %a, i8 %val, i32 100, i1 false)
  ret void
}

If I use llc ID/a.ll -mattr=+bulk-memory, It will use memory.fill but if I use llc ID/a.ll -mattr=-bulk-memory, it won't use any bulk memory instruction and disable this feature although it declared in function.

Which means it is impossible to have conflict between functions' feature and global feature.

Related code is in MCSubtargetInfo::InitMCProcessorInfo, here feature in opinion will be apply.

[tlively]

Looks good, thanks!