-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[WebAssembly] Split and tidy up target features test #96735
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This splits `target-features.ll` into two tests: `target-features-attrs.ll` and `target-features-cpus.ll`. Now `target-features-attrs.ll` contains tests with bitcode function attributes and `-mattr=` options. The current `target-features.ll` file's FileCheck lines are confusing, mainly because it is unclear how `CHECK` and `ATTRS` lines are meant to be different. Turns out, before llvm@67ec874, `-mattr=` options used to override any existing bitcode function attributes, but after the commit that's not the case anymore. So the original test had a line that tested `i32.atomic.rmw.cmpxchg` was not generated when `-mattr=+simd128` was given (because the existing `+atomics` in the function attributes is overriden). That commit deleted that line and changed some `ATTRS` lines into `CHECK`, which was confusing. This PR simplifies that part and does not test the absence of any instructions, and the effect of `-mattr=` option is only tested with the target features section. And `target-features-cpus.ll` only tests the sets of features enabled by `-mcpu=` lines. It is better to have this as a separate file because once you have bitcode function attributes they end up in the target features section too, making the testing of only the `-mcpu=` options difficult.
@llvm/pr-subscribers-backend-webassembly Author: Heejin Ahn (aheejin) ChangesThis splits Now And Full diff: https://github.com/llvm/llvm-project/pull/96735.diff 2 Files Affected:
diff --git a/llvm/test/CodeGen/WebAssembly/target-features-attrs.ll b/llvm/test/CodeGen/WebAssembly/target-features-attrs.ll
index 953a4ac9b41de..25dee51ac8c38 100644
--- a/llvm/test/CodeGen/WebAssembly/target-features-attrs.ll
+++ b/llvm/test/CodeGen/WebAssembly/target-features-attrs.ll
@@ -1,6 +1,5 @@
-; RUN: llc < %s | FileCheck %s --check-prefixes CHECK,ATTRS
-; RUN: llc < %s -mcpu=mvp -mattr=+simd128 | FileCheck %s --check-prefixes CHECK,SIMD128
-; RUN: llc < %s -mcpu=bleeding-edge | FileCheck %s --check-prefixes CHECK,BLEEDING-EDGE
+; RUN: llc -mcpu=mvp < %s | FileCheck %s
+; RUN: llc -mcpu=mvp -mattr=+simd128 < %s | FileCheck %s --check-prefixes SIMD128
; Test that codegen emits target features from the command line or
; function attributes correctly and that features are enabled for the
@@ -33,84 +32,53 @@ attributes #2 = { "target-features"="+reference-types" }
; CHECK-LABEL: fn_atomics:
; Expanded atomicrmw min
-; ATTRS: loop
+; CHECK: loop
; CHECK: i32.atomic.rmw.cmpxchg
-; ATTRS: end_loop
+; CHECK: end_loop
; nontrapping fptoint
; CHECK: i32.trunc_sat_f32_u
-; ATTRS: i32.store
+; CHECK: i32.store
; `fn_nontrapping_fptoint` should be the same as `fn_atomics`
; CHECK-LABEL: fn_nontrapping_fptoint:
; Expanded atomicrmw min
-; ATTRS: loop
+; CHECK: loop
; CHECK: i32.atomic.rmw.cmpxchg
-; ATTRS: end_loop
+; CHECK: end_loop
; nontrapping fptoint
; CHECK: i32.trunc_sat_f32_u
-; ATTRS: i32.store
+; CHECK: i32.store
+; Features in function attributes:
+; +atomics, +nontrapping-fptoint, +reference-types
; CHECK-LABEL: .custom_section.target_features,"",@
-
-; +atomics, +reference-types, +mutable-globals
-; ATTRS-NEXT: .int8 5
-; ATTRS-NEXT: .int8 43
-; ATTRS-NEXT: .int8 7
-; ATTRS-NEXT: .ascii "atomics"
-; ATTRS-NEXT: .int8 43
-; ATTRS-NEXT: .int8 15
-; ATTRS-NEXT: .ascii "mutable-globals"
-; ATTRS-NEXT: .int8 43
-; ATTRS-NEXT: .int8 19
-; ATTRS-NEXT: .ascii "nontrapping-fptoint"
-; ATTRS-NEXT: .int8 43
-; ATTRS-NEXT: .int8 15
-; ATTRS-NEXT: .ascii "reference-types"
-; ATTRS-NEXT: .int8 43
-; ATTRS-NEXT: .int8 8
-
+; CHECK-NEXT: .int8 3
+; CHECK-NEXT: .int8 43
+; CHECK-NEXT: .int8 7
+; CHECK-NEXT: .ascii "atomics"
+; CHECK-NEXT: .int8 43
+; CHECK-NEXT: .int8 19
+; CHECK-NEXT: .ascii "nontrapping-fptoint"
+; CHECK-NEXT: .int8 43
+; CHECK-NEXT: .int8 15
+; CHECK-NEXT: .ascii "reference-types"
+
+; Features in function attributes + features specified by -mattr= option:
; +atomics, +nontrapping-fptoint, +reference-types, +simd128
-; SIMD128-NEXT: .int8 4
-; SIMD128-NEXT: .int8 43
-; SIMD128-NEXT: .int8 7
-; SIMD128-NEXT: .ascii "atomics"
-; SIMD128-NEXT: .int8 43
-; SIMD128-NEXT: .int8 19
-; SIMD128-NEXT: .ascii "nontrapping-fptoint"
-; SIMD128-NEXT: .int8 43
-; SIMD128-NEXT: .int8 15
-; SIMD128-NEXT: .ascii "reference-types"
-; SIMD128-NEXT: .int8 43
-; SIMD128-NEXT: .int8 7
-; SIMD128-NEXT: .ascii "simd128"
-
-; +atomics, +bulk-memory, +mutable-globals, +nontrapping-fptoint,
-; +reference-types, +sign-ext, +simd128, +tail-call
-; BLEEDING-EDGE-NEXT: .int8 8
-; BLEEDING-EDGE-NEXT: .int8 43
-; BLEEDING-EDGE-NEXT: .int8 7
-; BLEEDING-EDGE-NEXT: .ascii "atomics"
-; BLEEDING-EDGE-NEXT: .int8 43
-; BLEEDING-EDGE-NEXT: .int8 11
-; BLEEDING-EDGE-NEXT: .ascii "bulk-memory"
-; BLEEDING-EDGE-NEXT: .int8 43
-; BLEEDING-EDGE-NEXT: .int8 15
-; BLEEDING-EDGE-NEXT: .ascii "mutable-globals"
-; 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 15
-; BLEEDING-EDGE-NEXT: .ascii "reference-types"
-; 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"
-; BLEEDING-EDGE-NEXT: .int8 43
-; BLEEDING-EDGE-NEXT: .int8 9
-; BLEEDING-EDGE-NEXT: .ascii "tail-call"
+; SIMD128-LABEL: .custom_section.target_features,"",@
+; SIMD128-NEXT: .int8 4
+; SIMD128-NEXT: .int8 43
+; SIMD128-NEXT: .int8 7
+; SIMD128-NEXT: .ascii "atomics"
+; SIMD128-NEXT: .int8 43
+; SIMD128-NEXT: .int8 19
+; SIMD128-NEXT: .ascii "nontrapping-fptoint"
+; SIMD128-NEXT: .int8 43
+; SIMD128-NEXT: .int8 15
+; SIMD128-NEXT: .ascii "reference-types"
+; SIMD128-NEXT: .int8 43
+; SIMD128-NEXT: .int8 7
+; SIMD128-NEXT: .ascii "simd128"
diff --git a/llvm/test/CodeGen/WebAssembly/target-features-cpus.ll b/llvm/test/CodeGen/WebAssembly/target-features-cpus.ll
new file mode 100644
index 0000000000000..d368271682a96
--- /dev/null
+++ b/llvm/test/CodeGen/WebAssembly/target-features-cpus.ll
@@ -0,0 +1,48 @@
+; RUN: llc < %s -mcpu=mvp | FileCheck %s --check-prefixes MVP
+; RUN: llc < %s -mcpu=generic | FileCheck %s --check-prefixes GENERIC
+; RUN: llc < %s | FileCheck %s --check-prefixes GENERIC
+; RUN: llc < %s -mcpu=bleeding-edge | FileCheck %s --check-prefixes BLEEDING-EDGE
+
+; Test that the target features section contains the correct set of features
+; depending on -mcpu= options.
+
+target triple = "wasm32-unknown-unknown"
+
+; mvp: should not contain the target features section
+; MVP-NOT: .custom_section.target_features,"",@
+
+; generic: +mutable-globals, +sign-ext
+; GENERIC-LABEL: .custom_section.target_features,"",@
+; GENERIC-NEXT: .int8 2
+; GENERIC-NEXT: .int8 43
+; GENERIC-NEXT: .int8 15
+; GENERIC-NEXT: .ascii "mutable-globals"
+; GENERIC-NEXT: .int8 43
+; GENERIC-NEXT: .int8 8
+; GENERIC-NEXT: .ascii "sign-ext"
+
+; bleeding-edge: +atomics, +bulk-memory, +mutable-globals, +nontrapping-fptoint,
+; +sign-ext, +simd128, +tail-call
+; BLEEDING-EDGE-LABEL: .section .custom_section.target_features,"",@
+; BLEEDING-EDGE-NEXT: .int8 7
+; BLEEDING-EDGE-NEXT: .int8 43
+; BLEEDING-EDGE-NEXT: .int8 7
+; BLEEDING-EDGE-NEXT: .ascii "atomics"
+; BLEEDING-EDGE-NEXT: .int8 43
+; BLEEDING-EDGE-NEXT: .int8 11
+; BLEEDING-EDGE-NEXT: .ascii "bulk-memory"
+; BLEEDING-EDGE-NEXT: .int8 43
+; BLEEDING-EDGE-NEXT: .int8 15
+; BLEEDING-EDGE-NEXT: .ascii "mutable-globals"
+; 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"
+; BLEEDING-EDGE-NEXT: .int8 43
+; BLEEDING-EDGE-NEXT: .int8 9
+; BLEEDING-EDGE-NEXT: .ascii "tail-call"
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/190/builds/749 Here is the relevant piece of the build log for the reference:
|
@llvm-ci The failure doesn't seem to be related to this PR. |
It is not, the build bot was running out of disk space, and I've since taken care of that issue. Sorry for the noise. |
This splits `target-features.ll` into two tests: `target-features-attrs.ll` and `target-features-cpus.ll`. Now `target-features-attrs.ll` contains tests with bitcode function attributes and `-mattr=` options. The current `target-features.ll` file's FileCheck lines are confusing, mainly because it is unclear how `CHECK` and `ATTRS` lines are meant to be different. Turns out, before llvm@67ec874, `-mattr=` options used to override any existing bitcode function attributes, but after the commit that's not the case anymore. So the original test had a line that tested `i32.atomic.rmw.cmpxchg` was not generated when `-mattr=+simd128` was given (because the existing `+atomics` in the function attributes is overriden). That commit deleted that line and changed some `ATTRS` lines into `CHECK`, which was confusing. This PR simplifies that part and does not test the absence of any instructions, and the effect of `-mattr=` option is only tested with the target features section. And `target-features-cpus.ll` only tests the sets of features enabled by `-mcpu=` lines. It is better to have this as a separate file because once you have bitcode function attributes they end up in the target features section too, making the testing of only the `-mcpu=` options difficult.
This splits `target-features.ll` into two tests: `target-features-attrs.ll` and `target-features-cpus.ll`. Now `target-features-attrs.ll` contains tests with bitcode function attributes and `-mattr=` options. The current `target-features.ll` file's FileCheck lines are confusing, mainly because it is unclear how `CHECK` and `ATTRS` lines are meant to be different. Turns out, before llvm@67ec874, `-mattr=` options used to override any existing bitcode function attributes, but after the commit that's not the case anymore. So the original test had a line that tested `i32.atomic.rmw.cmpxchg` was not generated when `-mattr=+simd128` was given (because the existing `+atomics` in the function attributes is overriden). That commit deleted that line and changed some `ATTRS` lines into `CHECK`, which was confusing. This PR simplifies that part and does not test the absence of any instructions, and the effect of `-mattr=` option is only tested with the target features section. And `target-features-cpus.ll` only tests the sets of features enabled by `-mcpu=` lines. It is better to have this as a separate file because once you have bitcode function attributes they end up in the target features section too, making the testing of only the `-mcpu=` options difficult.
This splits
target-features.ll
into two tests:target-features-attrs.ll
andtarget-features-cpus.ll
.Now
target-features-attrs.ll
contains tests with bitcode function attributes and-mattr=
options. The currenttarget-features.ll
file's FileCheck lines are confusing, mainly because it is unclear howCHECK
andATTRS
lines are meant to be different. Turns out, before 67ec874,-mattr=
options used to override any existing bitcode function attributes, but after the commit that's not the case anymore. So the original test had a line that testedi32.atomic.rmw.cmpxchg
was not generated when-mattr=+simd128
was given (because the existing+atomics
in the function attributes is overriden). That commit deleted that line and changed someATTRS
lines intoCHECK
, which was confusing. This PR simplifies that part and does not test the absence of any instructions, and the effect of-mattr=
option is only tested with the target features section.And
target-features-cpus.ll
only tests the sets of features enabled by-mcpu=
lines. It is better to have this as a separate file because once you have bitcode function attributes they end up in the target features section too, making the testing of only the-mcpu=
options difficult.