This is an archive of the discontinued LLVM Phabricator instance.

[mlir][spirv] NFC: Merge ArithToSPIRV pattern decl and definition
ClosedPublic

Authored by antiagainst on Aug 12 2023, 7:41 AM.

Details

Summary

This makes the code easier to search and read.

Diff Detail

Event Timeline

antiagainst created this revision.Aug 12 2023, 7:41 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 12 2023, 7:41 AM
antiagainst requested review of this revision.Aug 12 2023, 7:41 AM
kuhar accepted this revision.Aug 12 2023, 10:31 AM
This revision is now accepted and ready to land.Aug 12 2023, 10:31 AM
jpienaar added inline comments.
mlir/lib/Conversion/ArithToSPIRV/ArithToSPIRV.cpp
150

OOC why not use the standalone function form (https://github.com/llvm/llvm-project/commit/6874726610cc2f9eea7fa828c8585bf84969f9c3) here given no benefits specified?

kuhar added inline comments.Aug 12 2023, 12:05 PM
mlir/lib/Conversion/ArithToSPIRV/ArithToSPIRV.cpp
150

Cool, TIL!
Seems like this should be able to cover the vast majority of patterns but I haven't seen it used in MLIR/IREE, do you know the reasons for it?

antiagainst marked 2 inline comments as done.Aug 12 2023, 4:16 PM
antiagainst added inline comments.
mlir/lib/Conversion/ArithToSPIRV/ArithToSPIRV.cpp
150

Ha, interesting! Didn't notice that! However it does not support conversion patterns though?

antiagainst marked an inline comment as done.Aug 12 2023, 4:26 PM
antiagainst added inline comments.
mlir/lib/Conversion/ArithToSPIRV/ArithToSPIRV.cpp
150

I'll land this for now. We can further clean/tidy things later.