This is an archive of the discontinued LLVM Phabricator instance.

[mlir][spirv] Handle dynamic/static cases differntly for kernel capability
ClosedPublic

Authored by nirvedhmeshram on Sep 29 2022, 2:17 PM.

Details

Summary

This patch makes the SPIRVTypeConverter to account for the difference between static cases and dynamic cases for OpenCL.
Fixes https://github.com/llvm/llvm-project/issues/58058

Diff Detail

Event Timeline

nirvedhmeshram created this revision.Sep 29 2022, 2:17 PM
nirvedhmeshram requested review of this revision.Sep 29 2022, 2:17 PM
antiagainst requested changes to this revision.Sep 29 2022, 4:58 PM

Nice, thanks for fixing it! Just some minor issues.

mlir/lib/Dialect/SPIRV/Transforms/SPIRVConversion.cpp
346

Can we put comments here like "For OpenCL Kernel, dynamic shaped memrefs convert into a pointer pointing to the element.

And then move the comment at L341 down to before L348?

346

This can happen before L344? (So we don't need to build the arryType at all.)

408

Same here.

778

We can do the if-else just for the return builder.create apart? Concretely:

c++
Value linearIndex;
if (baseType...) {
  ...
} else {
  ...
}

Type pointeeType = basePtr.getType().cast<spirv::PointerType>().getPointeeType();
if (pointeeType.isa<spirv::ArrayType)
  return builder.create<spirv::AccessChainOp> ...
else
  return builder.create<spirv::PtrAccessChainOp> ...
This revision now requires changes to proceed.Sep 29 2022, 4:58 PM
nirvedhmeshram marked 4 inline comments as done.
antiagainst accepted this revision.Sep 29 2022, 7:28 PM
This revision is now accepted and ready to land.Sep 29 2022, 7:28 PM
This revision was landed with ongoing or failed builds.Sep 29 2022, 7:35 PM
This revision was automatically updated to reflect the committed changes.