[mlir][spirv] Let SPIRVConversionTarget consider type availability
ClosedPublic
Actions

Authored by antiagainst on Mar 9 2020, 2:10 PM.

Details

Reviewers

denis13
mravishankar
herhut

Commits

rG67e8690e53c3: [mlir][spirv] Let SPIRVConversionTarget consider type availability

Summary

Previously we only consider the version/extension/capability requirement
on the op itself. This commit updates SPIRVConversionTarget to also
take into consideration the values' types when deciding op legality.

Depends On D75875

Diff Detail

Repository: rG LLVM Github Monorepo

Event Timeline

antiagainst created this revision.Mar 9 2020, 2:10 PM

Herald added a reviewer: mravishankar. · View Herald TranscriptMar 9 2020, 2:10 PM

Herald added a reviewer: herhut. · View Herald Transcript

Herald added a project: Restricted Project. · View Herald Transcript

Herald added subscribers: llvm-commits, bader, Joonsoo and 11 others. · View Herald Transcript

Harbormaster failed remote builds in B48607: Diff 249221!Mar 9 2020, 3:07 PM

herhut resigned from this revision.Mar 12 2020, 4:30 AM

Updates

antiagainst added a child revision: D76242: [mlir][spirv] Plumbing target environment into type converter.Mar 16 2020, 11:51 AM

mravishankar requested changes to this revision.Mar 16 2020, 12:12 PM

mravishankar added inline comments.

mlir/lib/Dialect/SPIRV/SPIRVLowering.cpp
414	Could we add a method that sets some targetEnv by default (the one that are used in the tests here for example).

This revision now requires changes to proceed.Mar 16 2020, 12:12 PM

mravishankar added inline comments.Mar 16 2020, 12:15 PM

mlir/lib/Dialect/SPIRV/SPIRVLowering.cpp
414	Just to add to this. Then we can remove the "lookupTargetEnvOrDefault" method right?

Harbormaster failed remote builds in B49340: Diff 250605!Mar 16 2020, 12:35 PM

antiagainst edited parent revisions, added: D76241: [mlir][spirv] Use memref memory space for storage class; removed: D75875: [mlir][spirv] Support querying type extension/capability requirements.Mar 16 2020, 3:07 PM

antiagainst marked 3 inline comments as done.Mar 17 2020, 6:57 AM

antiagainst added inline comments.

mlir/lib/Dialect/SPIRV/SPIRVLowering.cpp
414	I'd like to keep the responsibility separate. We already have three helper functions for getting the target env: `getDefaultTargetEnv`, `lookupTargetEnv`, and `lookupTargetEnvOrDefault`. This class should be focusing on conversion target. If one want to just use the default target env, one can use `getDefaultTargetEnv` to create a `SPIRVConversionTarget`.