Skip to content

Commit

Permalink
[WebAssembly] Enable simd128 when relaxed-simd is set in AsmPrinter (#…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
aheejin authored and yuxuanchen1997 committed Jul 25, 2024
1 parent f1676d8 commit 4bc2c0f
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 1 deletion.
2 changes: 1 addition & 1 deletion llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
Original file line number Diff line number Diff line change
Expand Up @@ -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()">,
Expand Down
24 changes: 24 additions & 0 deletions llvm/test/CodeGen/WebAssembly/simd-asm-pred.ll
Original file line number Diff line number Diff line change
@@ -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
}

0 comments on commit 4bc2c0f

Please sign in to comment.