Page MenuHomePhabricator

[mlir] Fix for gpu-async-region pass.
ClosedPublic

Authored by csigg on Dec 11 2020, 10:35 PM.

Details

Summary
  • the !gpu.async.token is the second result of 'gpu.alloc async', not the first.
  • async.execute construction takes operand types not yet wrapped in !async.value.
  • fix typo

Diff Detail

Unit TestsFailed

TimeTest
60 msx64 windows > LLVM.CodeGen/XCore::threads.ll
Script: -- : 'RUN: at line 1'; c:\ws\w16n2-1\llvm-project\premerge-checks\build\bin\llc.exe -march=xcore < C:\ws\w16n2-1\llvm-project\premerge-checks\llvm\test\CodeGen\XCore\threads.ll | c:\ws\w16n2-1\llvm-project\premerge-checks\build\bin\filecheck.exe C:\ws\w16n2-1\llvm-project\premerge-checks\llvm\test\CodeGen\XCore\threads.ll

Event Timeline

csigg created this revision.Dec 11 2020, 10:35 PM
csigg requested review of this revision.Dec 11 2020, 10:35 PM

There should be some test that captures this.

mlir/lib/Dialect/GPU/Transforms/AsyncRegionRewriter.cpp
174

Should this be asserted in case the assumption changes?

csigg marked an inline comment as done.Dec 14 2020, 5:55 AM

There should be some test that captures this.

Done.

csigg updated this revision to Diff 311559.Dec 14 2020, 5:56 AM

Thanks for the review!

Apply comments.

herhut accepted this revision.Dec 15 2020, 1:28 AM

lgtm with extra test.

mlir/test/Dialect/GPU/async-region.mlir
30

Can you add an example where the async.execute returns a value beyond the token so that we can test that types get rewritten correctly?

This revision is now accepted and ready to land.Dec 15 2020, 1:28 AM
csigg updated this revision to Diff 311991.Dec 15 2020, 12:15 PM

Add test.

csigg marked an inline comment as done.Dec 15 2020, 12:16 PM
csigg updated this revision to Diff 312135.Dec 16 2020, 12:08 AM

Trigger pre-merge checks.

This revision was landed with ongoing or failed builds.Dec 16 2020, 10:08 AM
This revision was automatically updated to reflect the committed changes.