Skip to content
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

[MLIR][LLVM] Remove last remants of use-opaque-pointers from tests #71076

Merged
merged 1 commit into from
Nov 2, 2023

Conversation

Dinistro
Copy link
Contributor

@Dinistro Dinistro commented Nov 2, 2023

This commit removes the last remnants of use-opaque-pointers from the mlir tests. Two of the tests seem to be disabled, while the CUDA one is an integration test that didn't trigger a buildbot failure.

This commit removes the last remnants of `use-opaque-pointers` from
the mlir tests. Two of the tests seem to be disabled, while the CUDA one
is an integration test that didn't trigger a buildbot failure.
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 2, 2023

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-linalg

Author: Christian Ulmann (Dinistro)

Changes

This commit removes the last remnants of use-opaque-pointers from the mlir tests. Two of the tests seem to be disabled, while the CUDA one is an integration test that didn't trigger a buildbot failure.


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

3 Files Affected:

  • (modified) mlir/test/Conversion/FuncToLLVM/convert-data-layout.mlir (+1-1)
  • (modified) mlir/test/Dialect/Linalg/roundtrip.mlir (+1-1)
  • (modified) mlir/test/Integration/GPU/CUDA/sm90/tma_load_64x8_8x128_noswizzle.mlir (+1-1)
diff --git a/mlir/test/Conversion/FuncToLLVM/convert-data-layout.mlir b/mlir/test/Conversion/FuncToLLVM/convert-data-layout.mlir
index 0e7c16ec507998c..cb6bc35d452e1cf 100644
--- a/mlir/test/Conversion/FuncToLLVM/convert-data-layout.mlir
+++ b/mlir/test/Conversion/FuncToLLVM/convert-data-layout.mlir
@@ -1,6 +1,6 @@
 // RUN: mlir-opt -set-llvm-module-datalayout -convert-func-to-llvm %s | FileCheck %s
 
-// RUN-32: mlir-opt -set-llvm-module-datalayout='data-layout=p:32:32:32' -convert-func-to-llvm='use-opaque-pointers=1' %s \
+// RUN-32: mlir-opt -set-llvm-module-datalayout='data-layout=p:32:32:32' -convert-func-to-llvm %s \
 // RUN-32: | FileCheck %s
 
 // CHECK: module attributes {llvm.data_layout = ""}
diff --git a/mlir/test/Dialect/Linalg/roundtrip.mlir b/mlir/test/Dialect/Linalg/roundtrip.mlir
index 6203cf1c76d144c..b422066aade64f7 100644
--- a/mlir/test/Dialect/Linalg/roundtrip.mlir
+++ b/mlir/test/Dialect/Linalg/roundtrip.mlir
@@ -4,7 +4,7 @@
 // TODO: Re-enable LLVM lowering test.
 //
 // Test that we can lower all the way to LLVM without crashing, don't check results here.
-// DISABLED: mlir-opt %s -='use-opaque-pointers=1' -o=/dev/null 2>&1
+// DISABLED: mlir-opt %s -o=/dev/null 2>&1
 
 func.func @views(%arg0: index) {
   %c0 = arith.constant 0 : index
diff --git a/mlir/test/Integration/GPU/CUDA/sm90/tma_load_64x8_8x128_noswizzle.mlir b/mlir/test/Integration/GPU/CUDA/sm90/tma_load_64x8_8x128_noswizzle.mlir
index 44b127bd409ba62..081a60dded788a5 100644
--- a/mlir/test/Integration/GPU/CUDA/sm90/tma_load_64x8_8x128_noswizzle.mlir
+++ b/mlir/test/Integration/GPU/CUDA/sm90/tma_load_64x8_8x128_noswizzle.mlir
@@ -22,7 +22,7 @@
 // RUN:         -convert-vector-to-llvm \
 // RUN:         -convert-index-to-llvm=index-bitwidth=32 \
 // RUN:         -convert-arith-to-llvm \
-// RUN:         -finalize-memref-to-llvm='use-opaque-pointers=1' \
+// RUN:         -finalize-memref-to-llvm \
 // RUN:         -convert-func-to-llvm \
 // RUN:         -expand-strided-metadata --nvvm-attach-target="module=main_kernel features=+ptx80 chip=sm_90 O=3" \
 // RUN:  | mlir-opt -pass-pipeline='builtin.module(gpu.module(strip-debuginfo,convert-gpu-to-nvvm,convert-index-to-llvm{index-bitwidth=32},canonicalize,cse))' \

@llvmbot
Copy link
Collaborator

llvmbot commented Nov 2, 2023

@llvm/pr-subscribers-mlir-gpu

Author: Christian Ulmann (Dinistro)

Changes

This commit removes the last remnants of use-opaque-pointers from the mlir tests. Two of the tests seem to be disabled, while the CUDA one is an integration test that didn't trigger a buildbot failure.


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

3 Files Affected:

  • (modified) mlir/test/Conversion/FuncToLLVM/convert-data-layout.mlir (+1-1)
  • (modified) mlir/test/Dialect/Linalg/roundtrip.mlir (+1-1)
  • (modified) mlir/test/Integration/GPU/CUDA/sm90/tma_load_64x8_8x128_noswizzle.mlir (+1-1)
diff --git a/mlir/test/Conversion/FuncToLLVM/convert-data-layout.mlir b/mlir/test/Conversion/FuncToLLVM/convert-data-layout.mlir
index 0e7c16ec507998c..cb6bc35d452e1cf 100644
--- a/mlir/test/Conversion/FuncToLLVM/convert-data-layout.mlir
+++ b/mlir/test/Conversion/FuncToLLVM/convert-data-layout.mlir
@@ -1,6 +1,6 @@
 // RUN: mlir-opt -set-llvm-module-datalayout -convert-func-to-llvm %s | FileCheck %s
 
-// RUN-32: mlir-opt -set-llvm-module-datalayout='data-layout=p:32:32:32' -convert-func-to-llvm='use-opaque-pointers=1' %s \
+// RUN-32: mlir-opt -set-llvm-module-datalayout='data-layout=p:32:32:32' -convert-func-to-llvm %s \
 // RUN-32: | FileCheck %s
 
 // CHECK: module attributes {llvm.data_layout = ""}
diff --git a/mlir/test/Dialect/Linalg/roundtrip.mlir b/mlir/test/Dialect/Linalg/roundtrip.mlir
index 6203cf1c76d144c..b422066aade64f7 100644
--- a/mlir/test/Dialect/Linalg/roundtrip.mlir
+++ b/mlir/test/Dialect/Linalg/roundtrip.mlir
@@ -4,7 +4,7 @@
 // TODO: Re-enable LLVM lowering test.
 //
 // Test that we can lower all the way to LLVM without crashing, don't check results here.
-// DISABLED: mlir-opt %s -='use-opaque-pointers=1' -o=/dev/null 2>&1
+// DISABLED: mlir-opt %s -o=/dev/null 2>&1
 
 func.func @views(%arg0: index) {
   %c0 = arith.constant 0 : index
diff --git a/mlir/test/Integration/GPU/CUDA/sm90/tma_load_64x8_8x128_noswizzle.mlir b/mlir/test/Integration/GPU/CUDA/sm90/tma_load_64x8_8x128_noswizzle.mlir
index 44b127bd409ba62..081a60dded788a5 100644
--- a/mlir/test/Integration/GPU/CUDA/sm90/tma_load_64x8_8x128_noswizzle.mlir
+++ b/mlir/test/Integration/GPU/CUDA/sm90/tma_load_64x8_8x128_noswizzle.mlir
@@ -22,7 +22,7 @@
 // RUN:         -convert-vector-to-llvm \
 // RUN:         -convert-index-to-llvm=index-bitwidth=32 \
 // RUN:         -convert-arith-to-llvm \
-// RUN:         -finalize-memref-to-llvm='use-opaque-pointers=1' \
+// RUN:         -finalize-memref-to-llvm \
 // RUN:         -convert-func-to-llvm \
 // RUN:         -expand-strided-metadata --nvvm-attach-target="module=main_kernel features=+ptx80 chip=sm_90 O=3" \
 // RUN:  | mlir-opt -pass-pipeline='builtin.module(gpu.module(strip-debuginfo,convert-gpu-to-nvvm,convert-index-to-llvm{index-bitwidth=32},canonicalize,cse))' \

@@ -4,7 +4,7 @@
// TODO: Re-enable LLVM lowering test.
//
// Test that we can lower all the way to LLVM without crashing, don't check results here.
// DISABLED: mlir-opt %s -='use-opaque-pointers=1' -o=/dev/null 2>&1
// DISABLED: mlir-opt %s -o=/dev/null 2>&1
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This line looked already broken in main. Maybe we should just remove this run line + the TODO above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nicolasvasilache should we just remove this line in a followup patch?

Copy link
Member

@zero9178 zero9178 left a comment

Choose a reason for hiding this comment

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

LGTM

I don't really have a strong opinion about the odd linalg line but should probably be addressed in another PR either way

@Dinistro Dinistro merged commit 6cc1c2c into llvm:main Nov 2, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants