This is an archive of the discontinued LLVM Phabricator instance.

Create a gpu.module operation for the GPU Dialect.
ClosedPublic

Authored by tpopp on Jan 17 2020, 6:43 AM.

Details

Summary

This is based on the use of code constantly checking for an attribute on
a model and instead represents the distinct operaion with a different
op. Instead, this op can be used to provide better filtering.

Reverts "Revert "[mlir] Create a gpu.module operation for the GPU Dialect.""

This reverts commit ac446302ca4145cdc89f377c0c364c29ee303be5 after
fixing internal Google issues.

This additionally updates ROCDL lowering to use the new gpu.module.

Diff Detail

Event Timeline

tpopp created this revision.Jan 17 2020, 6:43 AM

Unit tests: pass. 61953 tests passed, 0 failed and 783 were skipped.

clang-tidy: unknown.

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Can you expand on the original reason for the revert? What has been fix in the patch?

tpopp updated this revision to Diff 239039.Jan 20 2020, 2:03 AM

Run clang-format with LLVM style instead of google style.

tpopp added a comment.Jan 20 2020, 2:09 AM

Can you expand on the original reason for the revert? What has been fix in the patch?

In the commit message or just in general? An XLA:MLIR test was failing when Google tried to update LLVM, so this was reverted. It took some time to figure out what patch is needed on the Tensorflow side to successfully integrate this patch because the style of failure was only caught by integration tests in a code base I didn't know.

Unit tests: pass. 61953 tests passed, 0 failed and 783 were skipped.

clang-tidy: unknown.

clang-format: pass.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

herhut accepted this revision.Jan 20 2020, 2:23 AM

Thanks.

This revision is now accepted and ready to land.Jan 20 2020, 2:23 AM

Can you expand on the original reason for the revert? What has been fix in the patch?

In the commit message or just in general? An XLA:MLIR test was failing when Google tried to update LLVM, so this was reverted. It took some time to figure out what patch is needed on the Tensorflow side to successfully integrate this patch because the style of failure was only caught by integration tests in a code base I didn't know.

OK, I expect that there is a strong suspicion of an upstream bug when reverting a commit ; "causing problem downstream" by itself isn't enough.

This revision was automatically updated to reflect the committed changes.