[PATCH] D73491: [mlir][spirv] Add GroupNonUniform arithmetic operations.

Lei Zhang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 28 06:41:12 PST 2020


antiagainst accepted this revision.
antiagainst added a comment.

Thanks Denis for adding these ops!



================
Comment at: mlir/lib/Dialect/SPIRV/SPIRVOps.cpp:631
+          << stringifyScope(static_cast<spirv::Scope>(
+                 groupOp->getAttrOfType<IntegerAttr>(kExecutionScopeAttrName)
+                     .getInt()))
----------------
denis13 wrote:
> denis13 wrote:
> > mravishankar wrote:
> > > One suggestion is to make this a templated method (templated on OpTy). Then you could use ".execution_scope()" and ".group_operation()" to get the attribute values
> > @mravishankar @antiagainst  I agree that templated methods look more readable, but on the other hand for each GroupNonUniform arithmetic operation will be created 3 methods with only one difference - function signature. They will implement the same logic and code.  Also I was looking on the other parser/printer/verifier functions for operations from the same category, for example `cast op`, and they use Operation* as function parameter, what do you think? Thanks.
> I was wrong about 3 methods, since parser is already generic, so it will be 2 methods for each GroupNonUniform arithmetic operation.
That's reasonable. This is a minor issue anyway; I'm fine with the current implementation. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73491/new/

https://reviews.llvm.org/D73491





More information about the llvm-commits mailing list