Page MenuHomePhabricator

[MLIR][SPIRV] Add rewrite pattern to convert select+cmp into GLSL clamp.
ClosedPublic

Authored by ergawy on Dec 21 2020, 12:34 AM.

Details

Summary

Adds rewrite patterns to convert select+cmp instructions into clamp
instructions whenever possible. Support is added to convert:

  • FOrdLessThan, FOrdLessThanEqual to GLSLFClampOp.
  • SLessThan, SLessThanEqual to GLSLSClampOp.
  • ULessThan, ULessThanEqual to GLSLUClampOp.

Diff Detail

Event Timeline

ergawy created this revision.Dec 21 2020, 12:34 AM
ergawy requested review of this revision.Dec 21 2020, 12:34 AM

2 things I still need to figure out/need to make sure I got right:

  • How to enable these patterns only when the GLSL.std.450 extension is enabled?
  • Whether you think it's suitable to consider these rewrites as canonicalization patterns or we should move them somewhere else?
mravishankar requested changes to this revision.Dec 21 2020, 1:44 PM
mravishankar added inline comments.
mlir/lib/Dialect/SPIRV/IR/SPIRVCanonicalization.cpp
239 ↗(On Diff #313032)

Nice! but I am not sure we want to add it for general canonicalizers. GLSL is an extension and you dont necessarily need to use it. Maybe put these patterns into another method

populateSPIRVGLSLCanonicalizationPatterns(...)

Then its upto client to use that. For testing this just create a simple pass that just runs those canonicalization patterns

This revision now requires changes to proceed.Dec 21 2020, 1:44 PM
ergawy updated this revision to Diff 313498.Dec 23 2020, 12:39 AM
  • Split GLSL-specific patterns in their own file.
  • Add a test pass to test them.
ergawy updated this revision to Diff 313500.Dec 23 2020, 12:45 AM

Fix file organization issue.

ergawy updated this revision to Diff 313502.Dec 23 2020, 12:47 AM

Remove empty line.

ergawy marked an inline comment as done.Dec 23 2020, 12:49 AM
ergawy added inline comments.
mlir/lib/Dialect/SPIRV/IR/SPIRVCanonicalization.cpp
239 ↗(On Diff #313032)

Thanks @mravishankar for the review. I re-organized the code according to your suggestion. Let me know if further changes need to be done here.

ergawy updated this revision to Diff 313503.Dec 23 2020, 1:11 AM
ergawy marked an inline comment as done.

Add missing header guard.

ergawy updated this revision to Diff 313506.Dec 23 2020, 1:30 AM

Handle some minor clang-tidy comments.

mravishankar accepted this revision.Dec 23 2020, 5:42 AM

Thanks! LGTM

This revision is now accepted and ready to land.Dec 23 2020, 5:42 AM