This is an archive of the discontinued LLVM Phabricator instance.

[mlir][gpu] Modifies `gpu.launch_func` to allow lowering it after gpu-to-llvm.
ClosedPublic

Authored by fmorac on Jun 29 2023, 12:49 PM.

Details

Summary

For an explanation of these patches see D154153.

Commit message:
In order to lower gpu.launch_func after running gpu-to-llvm it must be
able to handle lowered types -eg. index -> i64. This patch also allows the op
to refer to GPU binaries and not only GPU modules.

Depends on D154132.

Diff Detail

Event Timeline

fmorac created this revision.Jun 29 2023, 12:49 PM
fmorac updated this revision to Diff 535975.Jun 29 2023, 1:15 PM

Update dependencies.

fmorac edited the summary of this revision. (Show Details)Jun 29 2023, 5:46 PM
fmorac updated this revision to Diff 536074.Jun 29 2023, 5:47 PM

Rebasing.

fmorac edited the summary of this revision. (Show Details)Jun 29 2023, 6:19 PM
fmorac added a reviewer: mehdi_amini.
fmorac published this revision for review.Jun 30 2023, 6:30 AM

Comments wrt assembly format

mlir/lib/Dialect/GPU/IR/GPUDialect.cpp
1081

This is just AllTypesMatch in the tablegen

1092

I don't think this'll be needed with an AllTypesMatch

fmorac updated this revision to Diff 543192.Jul 22 2023, 8:02 AM

Added the AllTypesMatch trait.

Please add tests here

mlir/lib/Dialect/GPU/IR/GPUDialect.cpp
328–329

Update the comment?

1001–1039

Since we're touching the build methods, can you update these to use properties instead?

should look like:

Properties &prop = result.getOrAddProperties<Properties>();
prop.operand_segment_sizes = builder.getDenseI32ArrayAttr(segmentSizes);
fmorac added inline comments.Jul 24 2023, 5:32 AM
mlir/lib/Dialect/GPU/IR/GPUDialect.cpp
328–329

I'll update it.

1001–1039

Yes.

mehdi_amini added inline comments.Jul 24 2023, 11:06 PM
mlir/lib/Dialect/GPU/IR/GPUDialect.cpp
1001–1039

Actually prop.odsOperandSegmentSizes, I just changed it ;)

fmorac updated this revision to Diff 546568.Aug 2 2023, 12:09 PM
fmorac marked 5 inline comments as done.

Change builders to use properties.

Can you add tests for the new syntax and new types?

fmorac added a comment.Aug 2 2023, 7:12 PM

Can you add tests for the new syntax and new types?

Sure, I can add syntax & type tests here, however tests for verifying the compatibility with gpu.binary have to be added after the introduction of #gpu.select_object.

fmorac updated this revision to Diff 547741.Aug 7 2023, 5:32 AM

Added tests for the syntax.

mehdi_amini accepted this revision.Aug 8 2023, 9:58 PM
mehdi_amini added inline comments.
mlir/lib/Dialect/GPU/IR/GPUDialect.cpp
1039

Sorry, I removed the ods prefix...

This revision is now accepted and ready to land.Aug 8 2023, 9:58 PM
fmorac added inline comments.Aug 9 2023, 6:24 PM
mlir/lib/Dialect/GPU/IR/GPUDialect.cpp
1039

Is that change in trunk? Because I just did a build and it's still using odsOperandSegmentSizes.

mehdi_amini added inline comments.Aug 9 2023, 10:18 PM
mlir/lib/Dialect/GPU/IR/GPUDialect.cpp
1039

I forgot to push... now it is there :)

fmorac updated this revision to Diff 549330.Aug 11 2023, 4:06 AM

Removing Ods prefix.