From 4bc2c0f7bc65b92c3caac7519305c769aa5707e9 Mon Sep 17 00:00:00 2001
From: Heejin Ahn <aheejin@gmail.com>
Date: Tue, 23 Jul 2024 11:50:56 -0700
Subject: [PATCH] [WebAssembly] Enable simd128 when relaxed-simd is set in
 AsmPrinter (#99803)

Summary:
Even though in `Subtarget` we defined `SIMDLevel` as a number so
`hasRelaxedSIMD` automatically means `hasSIMD128`,
https://github.com/llvm/llvm-project/blob/0caf0c93e759816663af52e8632d1c3953dbc715/llvm/lib/Target/WebAssembly/WebAssemblySubtarget.h#L36-L40
https://github.com/llvm/llvm-project/blob/0caf0c93e759816663af52e8632d1c3953dbc715/llvm/lib/Target/WebAssembly/WebAssemblySubtarget.h#L107

specifying only `relaxed-simd` feature on a program that needs `simd128`
instructions to compile fails, because of this query in `AsmPrinter`:
https://github.com/llvm/llvm-project/blob/d0d05aec3b6792136a9f75eb85dd2ea66005ae12/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp#L644-L645

This `verifyInstructionPredicates` function (and other functions called
by this function) is generated by

https://github.com/llvm/llvm-project/blob/main/llvm/utils/TableGen/InstrInfoEmitter.cpp,
and looks like this (you can check it in the
`lib/Target/WebAssembly/WebAssemblyGenInstrInfo.inc` in your build
directory):
```cpp
void verifyInstructionPredicates(
    unsigned Opcode, const FeatureBitset &Features) {
  FeatureBitset AvailableFeatures = computeAvailableFeatures(Features);
  FeatureBitset RequiredFeatures = computeRequiredFeatures(Opcode);
  FeatureBitset MissingFeatures =
      (AvailableFeatures & RequiredFeatures) ^
      RequiredFeatures;
  ...
}
```

And `computeAvailableFeatures` is just a set query, like this:
```cpp
inline FeatureBitset computeAvailableFeatures(const FeatureBitset &FB) {
  FeatureBitset Features;
  if (FB[WebAssembly::FeatureAtomics])
    Features.set(Feature_HasAtomicsBit);
  if (FB[WebAssembly::FeatureBulkMemory])
    Features.set(Feature_HasBulkMemoryBit);
  if (FB[WebAssembly::FeatureExceptionHandling])
    Features.set(Feature_HasExceptionHandlingBit);
  ...
```

So this is how currently `HasSIMD128` is defined:

https://github.com/llvm/llvm-project/blob/0caf0c93e759816663af52e8632d1c3953dbc715/llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td#L79-L81

The things being checked in this `computeAvailableFeatures`, and in turn
in `AsmPrinter`, are `AssemblerPredicate`s. These only check which bits
are set in the features set and are different from `Predicate`s, which
can call `Subtarget` functions like `Subtarget->hasSIMD128()`.

But apparently we can use `all_of` and `any_of` directives in
`AssemblerPredicate`, and we can make `simd128`'s `AssemblerPredicate`
set in `relaxed-simd` is set by the condition as an 'or' of the two.

Fixes #98502.

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:


Differential Revision: https://phabricator.intern.facebook.com/D60250795
---
 .../WebAssembly/WebAssemblyInstrInfo.td       |  2 +-
 .../test/CodeGen/WebAssembly/simd-asm-pred.ll | 24 +++++++++++++++++++
 2 files changed, 25 insertions(+), 1 deletion(-)
 create mode 100644 llvm/test/CodeGen/WebAssembly/simd-asm-pred.ll

diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td b/llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
index 3d37eb2fa27bce..bb36ce7650183f 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
@@ -78,7 +78,7 @@ def HasSignExt :
 
 def HasSIMD128 :
     Predicate<"Subtarget->hasSIMD128()">,
-    AssemblerPredicate<(all_of FeatureSIMD128), "simd128">;
+    AssemblerPredicate<(any_of FeatureSIMD128, FeatureRelaxedSIMD), "simd128">;
 
 def HasTailCall :
     Predicate<"Subtarget->hasTailCall()">,
diff --git a/llvm/test/CodeGen/WebAssembly/simd-asm-pred.ll b/llvm/test/CodeGen/WebAssembly/simd-asm-pred.ll
new file mode 100644
index 00000000000000..f022c3e745125f
--- /dev/null
+++ b/llvm/test/CodeGen/WebAssembly/simd-asm-pred.ll
@@ -0,0 +1,24 @@
+; RUN: llc < %s -verify-machineinstrs -mattr=+relaxed-simd | FileCheck %s
+
+; Test that setting "relaxed-simd" target feature set also implies 'simd128' in
+; AssemblerPredicate, which is used to verify instructions in AsmPrinter.
+
+target triple = "wasm32-unknown-unknown"
+
+declare <2 x i64> @llvm.wasm.relaxed.laneselect.v2i64(<2 x i64>, <2 x i64>, <2 x i64>)
+
+; The compiled result of this function uses LOCAL_GET_V128, which is predicated
+; on the 'simd128' feature. We should be able to compile this when only
+; 'relaxed-simd' is set, which implies 'simd128'.
+define <2 x i64> @test(<2 x i64>, <2 x i64>, <2 x i64>) #0 {
+; CHECK-LABEL: test:
+; CHECK:         .functype  test (v128, v128, v128) -> (v128)
+; CHECK-NEXT:  # %bb.0:
+; CHECK-NEXT:    local.get  0
+; CHECK-NEXT:    local.get  1
+; CHECK-NEXT:    local.get  2
+; CHECK-NEXT:    i64x2.relaxed_laneselect
+start:
+  %_4 = tail call <2 x i64> @llvm.wasm.relaxed.laneselect.v2i64(<2 x i64> %0, <2 x i64> %1, <2 x i64> %2) #3
+  ret <2 x i64> %_4
+}