Skip to content

[WebAssembly] Fix uses of -DAG and -NOT in wasm-target-features.c #89777

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

Merged
merged 1 commit into from
Apr 24, 2024

Conversation

aheejin
Copy link
Member

@aheejin aheejin commented Apr 23, 2024

We are currently using PREFIX-DAG and PREFIX-NOT within a single PREFIX test in a mixed way, but -DAG and -NOT do not work that way. For example:

Result:

1
2
3

Test file:

// CHECK-DAG: 3
// CHECK-DAG: 1
// CHECK-NOT: 2

This does not work. The last line CHECK-NOT: 2 does not trigger any error, because we've already covered all three lines (1~3) while matching CHECK-DAG: 3 and CHECK-DAG: 1, and FileCheck tries to check the line CHECK-NOT: 2 after the line 3.

Actually, we have

// BLEEDING-EDGE-NOT:#define __wasm_reference_types__ 1{{$}}

even though reference-types is enabled in 'bleeding-edge' config, and this has not triggered any error.

This section (https://llvm.org/docs/CommandGuide/FileCheck.html#the-check-dag-directive) explains the interactions between CHECK-DAG and CHECK-NOTs:

As a result, the surrounding CHECK-DAG: directives cannot be reordered, i.e. all occurrences matching CHECK-DAG: before CHECK-NOT: must not fall behind occurrences matching CHECK-DAG: after CHECK-NOT:.

So in order to test the 'include' lists and 'not-include' lists, we have to run the tests twice with different prefixes. This splits GENERIC and BLEEDING-EDGE tests in two configs (***-INCLUDE and ***) to test them correctly.

This also adds some spaces after colons, sorts the feature lists, and adds 1{{$}} to the MVP tests to make them consistent with GENERIC and BLEEDING-EDGE tests.

We are currently using `PREFIX-DAG` and `PREFIX-NOT` within a single
`PREFIX` test in a mixed way, but `-DAG` and `-NOT` do not work that
way. For example:

Result:
```
1
2
3
```

Test file:
```c
// CHECK-DAG: 3
// CHECK-DAG: 1
// CHECK-NOT: 2
```

This does not work. The last line `CHECK-NOT: 2` does not trigger any
error, because we've already covered all three lines (1~3) while
matching `CHECK-DAG: 3` and `CHECK-DAG: 1`, and FileCheck tries to check
the line `CHECK-NOT: 2` _after_ the line `3`.

Actually, we have
```c
// BLEEDING-EDGE-NOT:#define __wasm_reference_types__ 1{{$}}
```
even though reference-types is enabled in 'bleeding-edge' config, and
this has not triggered any error.

This section
(https://llvm.org/docs/CommandGuide/FileCheck.html#the-check-dag-directive)
explains the interactions between `CHECK-DAG` and `CHECK-NOT`s:
> As a result, the surrounding `CHECK-DAG:` directives cannot be
> reordered, i.e. all occurrences matching `CHECK-DAG:` before
> `CHECK-NOT:` must not fall behind occurrences matching `CHECK-DAG:`
> after `CHECK-NOT:`.

So in order to test the 'include' lists and 'not-include' lists, we have
to run the tests twice with different prefixes. This splits `GENERIC`
and `BLEEDING-EDGE` tests in two configs (`***-INCLUDE` and `***`) to
test them correctly.

This also adds some spaces after colons, sorts the feature lists, and
adds `1{{$}}` to the `MVP` tests to make them consistent with `GENERIC`
and `BLEEDING-EDGE` tests.
@aheejin aheejin requested review from sunfishcode and tlively April 23, 2024 15:14
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Apr 23, 2024
@llvmbot
Copy link
Member

llvmbot commented Apr 23, 2024

@llvm/pr-subscribers-clang

Author: Heejin Ahn (aheejin)

Changes

We are currently using PREFIX-DAG and PREFIX-NOT within a single PREFIX test in a mixed way, but -DAG and -NOT do not work that way. For example:

Result:

1
2
3

Test file:

// CHECK-DAG: 3
// CHECK-DAG: 1
// CHECK-NOT: 2

This does not work. The last line CHECK-NOT: 2 does not trigger any error, because we've already covered all three lines (1~3) while matching CHECK-DAG: 3 and CHECK-DAG: 1, and FileCheck tries to check the line CHECK-NOT: 2 after the line 3.

Actually, we have

// BLEEDING-EDGE-NOT:#define __wasm_reference_types__ 1{{$}}

even though reference-types is enabled in 'bleeding-edge' config, and this has not triggered any error.

This section (https://llvm.org/docs/CommandGuide/FileCheck.html#the-check-dag-directive) explains the interactions between CHECK-DAG and CHECK-NOTs:
> As a result, the surrounding CHECK-DAG: directives cannot be reordered, i.e. all occurrences matching CHECK-DAG: before CHECK-NOT: must not fall behind occurrences matching CHECK-DAG: after CHECK-NOT:.

So in order to test the 'include' lists and 'not-include' lists, we have to run the tests twice with different prefixes. This splits GENERIC and BLEEDING-EDGE tests in two configs (***-INCLUDE and ***) to test them correctly.

This also adds some spaces after colons, sorts the feature lists, and adds 1{{$}} to the MVP tests to make them consistent with GENERIC and BLEEDING-EDGE tests.


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

1 Files Affected:

  • (modified) clang/test/Preprocessor/wasm-target-features.c (+55-38)
diff --git a/clang/test/Preprocessor/wasm-target-features.c b/clang/test/Preprocessor/wasm-target-features.c
index eccd432aa8eee6..983cd01cf8117e 100644
--- a/clang/test/Preprocessor/wasm-target-features.c
+++ b/clang/test/Preprocessor/wasm-target-features.c
@@ -132,20 +132,30 @@
 // RUN:     -target wasm64-unknown-unknown -mcpu=mvp \
 // RUN:   | FileCheck %s -check-prefix=MVP
 //
-// MVP-NOT:#define __wasm_simd128__
-// MVP-NOT:#define __wasm_nontrapping_fptoint__
-// MVP-NOT:#define __wasm_sign_ext__
-// MVP-NOT:#define __wasm_exception_handling__
-// MVP-NOT:#define __wasm_bulk_memory__
-// MVP-NOT:#define __wasm_atomics__
-// MVP-NOT:#define __wasm_mutable_globals__
-// MVP-NOT:#define __wasm_multivalue__
-// MVP-NOT:#define __wasm_tail_call__
-// MVP-NOT:#define __wasm_reference_types__
-// MVP-NOT:#define __wasm_extended_const__
-// MVP-NOT:#define __wasm_multimemory__
-// MVP-NOT:#define __wasm_relaxed_simd__
+// MVP-NOT: #define __wasm_atomics__ 1{{$}}
+// MVP-NOT: #define __wasm_bulk_memory__ 1{{$}}
+// MVP-NOT: #define __wasm_exception_handling__ 1{{$}}
+// MVP-NOT: #define __wasm_extended_const__ 1{{$}}
+// MVP-NOT: #define __wasm_multimemory__ 1{{$}}
+// MVP-NOT: #define __wasm_multivalue__ 1{{$}}
+// MVP-NOT: #define __wasm_mutable_globals__ 1{{$}}
+// MVP-NOT: #define __wasm_nontrapping_fptoint__ 1{{$}}
+// MVP-NOT: #define __wasm_reference_types__ 1{{$}}
+// MVP-NOT: #define __wasm_relaxed_simd__ 1{{$}}
+// MVP-NOT: #define __wasm_sign_ext__ 1{{$}}
+// MVP-NOT: #define __wasm_simd128__ 1{{$}}
+// MVP-NOT: #define __wasm_tail_call__ 1{{$}}
 
+// RUN: %clang -E -dM %s -o - 2>&1 \
+// RUN:     -target wasm32-unknown-unknown -mcpu=generic \
+// RUN:   | FileCheck %s -check-prefix=GENERIC-INCLUDE
+// RUN: %clang -E -dM %s -o - 2>&1 \
+// RUN:     -target wasm64-unknown-unknown -mcpu=generic \
+// RUN:   | FileCheck %s -check-prefix=GENERIC-INCLUDE
+//
+// GENERIC-INCLUDE-DAG: #define __wasm_mutable_globals__ 1{{$}}
+// GENERIC-INCLUDE-DAG: #define __wasm_sign_ext__ 1{{$}}
+//
 // RUN: %clang -E -dM %s -o - 2>&1 \
 // RUN:     -target wasm32-unknown-unknown -mcpu=generic \
 // RUN:   | FileCheck %s -check-prefix=GENERIC
@@ -153,19 +163,35 @@
 // RUN:     -target wasm64-unknown-unknown -mcpu=generic \
 // RUN:   | FileCheck %s -check-prefix=GENERIC
 //
-// GENERIC-DAG:#define __wasm_sign_ext__ 1{{$}}
-// GENERIC-DAG:#define __wasm_mutable_globals__ 1{{$}}
-// GENERIC-NOT:#define __wasm_nontrapping_fptoint__ 1{{$}}
-// GENERIC-NOT:#define __wasm_bulk_memory__ 1{{$}}
-// GENERIC-NOT:#define __wasm_simd128__ 1{{$}}
-// GENERIC-NOT:#define __wasm_atomics__ 1{{$}}
-// GENERIC-NOT:#define __wasm_tail_call__ 1{{$}}
-// GENERIC-NOT:#define __wasm_multimemory__ 1{{$}}
-// GENERIC-NOT:#define __wasm_exception_handling__ 1{{$}}
-// GENERIC-NOT:#define __wasm_multivalue__ 1{{$}}
-// GENERIC-NOT:#define __wasm_reference_types__ 1{{$}}
-// GENERIC-NOT:#define __wasm_extended_const__ 1{{$}}
+// GENERIC-NOT: #define __wasm_atomics__ 1{{$}}
+// GENERIC-NOT: #define __wasm_bulk_memory__ 1{{$}}
+// GENERIC-NOT: #define __wasm_exception_handling__ 1{{$}}
+// GENERIC-NOT: #define __wasm_extended_const__ 1{{$}}
+// GENERIC-NOT: #define __wasm_multimemory__ 1{{$}}
+// GENERIC-NOT: #define __wasm_multivalue__ 1{{$}}
+// GENERIC-NOT: #define __wasm_nontrapping_fptoint__ 1{{$}}
+// GENERIC-NOT: #define __wasm_reference_types__ 1{{$}}
+// GENERIC-NOT: #define __wasm_relaxed_simd__ 1{{$}}
+// GENERIC-NOT: #define __wasm_simd128__ 1{{$}}
+// GENERIC-NOT: #define __wasm_tail_call__ 1{{$}}
 
+// RUN: %clang -E -dM %s -o - 2>&1 \
+// RUN:     -target wasm32-unknown-unknown -mcpu=bleeding-edge \
+// RUN:   | FileCheck %s -check-prefix=BLEEDING-EDGE-INCLUDE
+// RUN: %clang -E -dM %s -o - 2>&1 \
+// RUN:     -target wasm64-unknown-unknown -mcpu=bleeding-edge \
+// RUN:   | FileCheck %s -check-prefix=BLEEDING-EDGE-INCLUDE
+//
+// BLEEDING-EDGE-INCLUDE-DAG: #define __wasm_atomics__ 1{{$}}
+// BLEEDING-EDGE-INCLUDE-DAG: #define __wasm_bulk_memory__ 1{{$}}
+// BLEEDING-EDGE-INCLUDE-DAG: #define __wasm_multimemory__ 1{{$}}
+// BLEEDING-EDGE-INCLUDE-DAG: #define __wasm_mutable_globals__ 1{{$}}
+// BLEEDING-EDGE-INCLUDE-DAG: #define __wasm_nontrapping_fptoint__ 1{{$}}
+// BLEEDING-EDGE-INCLUDE-DAG: #define __wasm_reference_types__ 1{{$}}
+// BLEEDING-EDGE-INCLUDE-DAG: #define __wasm_sign_ext__ 1{{$}}
+// BLEEDING-EDGE-INCLUDE-DAG: #define __wasm_simd128__ 1{{$}}
+// BLEEDING-EDGE-INCLUDE-DAG: #define __wasm_tail_call__ 1{{$}}
+//
 // RUN: %clang -E -dM %s -o - 2>&1 \
 // RUN:     -target wasm32-unknown-unknown -mcpu=bleeding-edge \
 // RUN:   | FileCheck %s -check-prefix=BLEEDING-EDGE
@@ -173,19 +199,10 @@
 // RUN:     -target wasm64-unknown-unknown -mcpu=bleeding-edge \
 // RUN:   | FileCheck %s -check-prefix=BLEEDING-EDGE
 //
-// BLEEDING-EDGE-DAG:#define __wasm_nontrapping_fptoint__ 1{{$}}
-// BLEEDING-EDGE-DAG:#define __wasm_sign_ext__ 1{{$}}
-// BLEEDING-EDGE-DAG:#define __wasm_bulk_memory__ 1{{$}}
-// BLEEDING-EDGE-DAG:#define __wasm_simd128__ 1{{$}}
-// BLEEDING-EDGE-DAG:#define __wasm_atomics__ 1{{$}}
-// BLEEDING-EDGE-DAG:#define __wasm_mutable_globals__ 1{{$}}
-// BLEEDING-EDGE-DAG:#define __wasm_tail_call__ 1{{$}}
-// BLEEDING-EDGE-DAG:#define __wasm_multimemory__ 1{{$}}
-// BLEEDING-EDGE-NOT:#define __wasm_exception_handling__ 1{{$}}
-// BLEEDING-EDGE-NOT:#define __wasm_multivalue__ 1{{$}}
-// BLEEDING-EDGE-NOT:#define __wasm_reference_types__ 1{{$}}
-// BLEEDING-EDGE-NOT:#define __wasm_extended_const__ 1{{$}}
-// BLEEDING-EDGE-NOT:#define __wasm_relaxed_simd__ 1{{$}}
+// BLEEDING-EDGE-NOT: #define __wasm_exception_handling__ 1{{$}}
+// BLEEDING-EDGE-NOT: #define __wasm_extended_const__ 1{{$}}
+// BLEEDING-EDGE-NOT: #define __wasm_multivalue__ 1{{$}}
+// BLEEDING-EDGE-NOT: #define __wasm_relaxed_simd__ 1{{$}}
 
 // RUN: %clang -E -dM %s -o - 2>&1 \
 // RUN:     -target wasm32-unknown-unknown -mcpu=bleeding-edge -mno-simd128 \

Copy link
Collaborator

@tlively tlively left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@aheejin aheejin merged commit c8c1e4e into llvm:main Apr 24, 2024
6 checks passed
@aheejin aheejin deleted the target_feature_test_fix branch April 24, 2024 05:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants