This is an archive of the discontinued LLVM Phabricator instance.

[mlir][spirv] Add a pass to deduce version/extension/capability
ClosedPublic

Authored by antiagainst on Mar 9 2020, 1:57 PM.

Details

Summary

Creates an operation pass that deduces and attaches the minimal version/
capabilities/extensions requirements for spv.module ops.

For each spv.module op, this pass requires a spv.target_env attribute on
it or an enclosing module-like op to drive the deduction. The reason is
that an op can be enabled by multiple extensions/capabilities. So we need
to know which one to pick. spv.target_env gives the hard limit as for
what the target environment can support; this pass deduces what are
actually needed for a specific spv.module op.

Depends On D75869

Diff Detail

Event Timeline

antiagainst created this revision.Mar 9 2020, 1:57 PM
Herald added a project: Restricted Project. · View Herald Transcript
mravishankar requested changes to this revision.Mar 10 2020, 11:40 AM
mravishankar added inline comments.
mlir/include/mlir/Dialect/SPIRV/Passes.h
37

This seems to assume that there is a "linear" order of capabilities and extensions. Is that the case?
Also, is there a way to have a "Base" limit you can start with? It can be the most lenient or the least lenient set

mlir/lib/Dialect/SPIRV/TargetAndABI.cpp
298

Nit: To me reading this as a while loop is easier, but I dont know what the convention is

This revision now requires changes to proceed.Mar 10 2020, 11:40 AM
antiagainst marked 3 inline comments as done.

Address comments

antiagainst added inline comments.Mar 11 2020, 1:05 PM
mlir/include/mlir/Dialect/SPIRV/Passes.h
37

For an op, there can exists multiple capabilities to enable it. E.g., OpGroupNonUniformIAdd can be enabled by any of [GroupNonUniformArithmetic, GroupNonUniformClustered, GroupNonUniformPartitionedNV]. It's a set (although under the hood we indeed search linearly but that's implementation detail). Any one of them allowed by the target it should be fine and we'll pick that one.

It's hard to find a more "general" or well supported one that's also a moving target given new hardware coming up and existing ones updated.

mravishankar accepted this revision.Mar 12 2020, 2:27 PM
This revision is now accepted and ready to land.Mar 12 2020, 2:27 PM
rriddle added inline comments.Mar 12 2020, 3:14 PM
mlir/lib/Dialect/SPIRV/Transforms/UpdateVCEPass.cpp
76

nit: You can return the error directly. LogicalResult converts to WalkResult, (failure -> interrupt, success -> advance).

122

nit: Move the comment before the variable.

antiagainst marked 3 inline comments as done.Mar 12 2020, 4:17 PM
antiagainst added inline comments.
mlir/lib/Dialect/SPIRV/Transforms/UpdateVCEPass.cpp
122

I think this way it reads more naturally.

This revision was automatically updated to reflect the committed changes.
antiagainst marked an inline comment as done.