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

Add level-zero-opt tool #264

Merged
merged 16 commits into from
Aug 2, 2022
Merged

Add level-zero-opt tool #264

merged 16 commits into from
Aug 2, 2022

Conversation

nbpatel
Copy link
Contributor

@nbpatel nbpatel commented Jul 16, 2022

Please review these guidelines to help with the review process:

  • Have you provided a meaningful PR description?
  • Have you added a test, a reproducer, or a reference to an issue with a reproducer?
  • Have you tested your changes locally for CPU and GPU devices?
  • Have you made sure that new changes do not introduce compiler warnings?
  • If this PR is a work in progress, are you filing the PR as a draft?
  • Have you organized your commits logically and ensured each can be built by itself?

This PR adds a directory structure to the mlir/test directory which is similar to upstream dir structure.
It adds a tool called level-zero-opt which allows us to run individual pass from command line and also adds a few test cases for doing some basic IR tests.
More test cases to be added.

@nbpatel nbpatel changed the title Nishant l0 opt Add level-zero-opt tool Jul 16, 2022
@nbpatel nbpatel requested review from silee2 and Hardcode84 July 16, 2022 22:31
@nbpatel nbpatel marked this pull request as ready for review July 19, 2022 16:54
}
func.func private @fillResource1DFloat(memref<?xf32>, f32)
func.func private @printMemrefF32(memref<*xf32>)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Newline at the end of the file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

// CHECK: spv.Branch ^bb1
// CHECK: ^bb1: // pred: ^bb0
// CHECK: %__builtin_var_WorkgroupId___addr = spv.mlir.addressof @__builtin_var_WorkgroupId__ : !spv.ptr<vector<3xi64>, Input>
// CHECK: %0 = spv.Load "Input" %__builtin_var_WorkgroupId___addr : vector<3xi64>
Copy link
Contributor

Choose a reason for hiding this comment

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

Generated names like %0 can be unstable between MLIR versions, please use %[[VAR1:.*]] = spv.Load ... %[[VAR2:.*]] = spv.CompositeExtract %[[VAR1]][0 : i32] name matching style

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

%2 = memref.alloc() : memref<8xf32>
// CHECK: %memref = gpu.alloc () : memref<8xf32>
// CHECK: %memref_0 = gpu.alloc () : memref<8xf32>
// CHECK: %memref_1 = gpu.alloc () : memref<8xf32>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please check there are actually loads from these gpu allocs inside gpu.launch

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what do you mean?

Copy link
Contributor

Choose a reason for hiding this comment

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

Something like

// CHECK: %[[MEMREF0:.*]]= gpu.alloc  () : memref<8xf32>
// CHECK: gpu.launch
// CHECK: memref.load %[[MEMREF0]][...]
// CHECK: gpu.terminator

Copy link
Contributor

Choose a reason for hiding this comment

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

@nbpatel please address this comment and then it is LGTM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

This is revised. Please let me know if further changes are required. -Liangliang

@Hardcode84
Copy link
Contributor

@nbpatel speaking of enabling/disabling individual lit tests, please take a look https://llvm.org/docs/TestingGuide.html#constraining-test-execution

Copy link
Contributor

@fschlimb fschlimb left a comment

Choose a reason for hiding this comment

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

Please make sure tests are actually run (and succeed) in CI.
You might want to look at refactor branch.

@lchang552
Copy link
Contributor

Please make sure tests are actually run (and succeed) in CI. You might want to look at refactor branch.
Run cd _build || exit 1
[0/1] cd /home/runner/work/mlir-extensions/mlir-extensions/_build/mlir/test && /usr/share/miniconda/bin/python3.9 /usr/share/miniconda/bin/lit -sv /home/runner/work/mlir-extensions/mlir-extensions/_build/mlir/test
-- Testing: 5 tests, 2 workers --
Testing: 0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..

Testing Time: 0.31s
Unsupported: 3
Passed : 2

Hi, The tests can run correctly if you check the output of the CI for the latest commit. Three are unsupported since they are end-to-end tests and the CI is not with the target GPU.

@fschlimb
Copy link
Contributor

fschlimb commented Aug 2, 2022

Three are unsupported since they are end-to-end tests and the CI is not with the target GPU.

I do not understand the numbers reported. Which are the ones which pass, and which don't?

@fschlimb
Copy link
Contributor

fschlimb commented Aug 2, 2022

ok, I see now, my bad. I had missed the "mlir" in the path.

Am I right that in the CI

  • the tests in /test are never run
  • l0-runner tests are never run
  • all tests for l0-opt are run and pass

?

@lchang552
Copy link
Contributor

Hi, yes, you are right. This commit currently only enables tests in mlir/test/Conversion and mlir/test/Transform, others are disabled. The numbers reported are only for tests in mlir/test.

@nbpatel nbpatel merged commit d2ab9dc into main Aug 2, 2022
@nbpatel nbpatel deleted the nishant_l0-opt branch August 3, 2022 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants