Making dialect names consistent

28 views
Skip to first unread message

Alex Zinenko

unread,
Oct 1, 2019, 5:01:23 AM10/1/19
to MLIR
Hi there,

I noticed that some of the dialects are suffixed with "Ops" and some others are not. Furthermore, the suffix appears only on some parts of the respective file paths, e.g. lib/Dialect/SPIRV/SPIRVOps.cpp vs lib/Dialect/AffineOps/AffineOps.cpp.  It would be great to have a more consistent approach and avoid thinking whether the "Ops" suffix is necessary for every dialect or include file.

My proposition would be to drop the suffix, with the following arguments.
- Dialects contain more than ops (types and attributes as well), some of them actually don't contain any ops and provide attributes for use with other dialects.
- We refer to "standard dialect" or "affine dialect" rather than "standard ops dialect" and "affine ops dialect".
- The suffixing was useful when dialect code was located under top-level lib/ to differentiate between dialects and other libraries; with dialects in lib/Dialect/, it became redundant. 

--
-- Alex

Lei Zhang

unread,
Oct 1, 2019, 10:38:32 AM10/1/19
to Alex Zinenko, MLIR
+1. I also think it's more consistent to have dialect directory names without "Ops" suffix. But for the source files inside each dialect's directory, it depends on the contents inside. For example, in SPIR-V dialect we have different source files for the dialect itself (SPIRVDialect.cpp), types (SPIRVTypes.cpp), and ops (SPIRVOps.cpp). 

Thanks,
Lei


--
You received this message because you are subscribed to the Google Groups "MLIR" group.
To unsubscribe from this group and stop receiving emails from it, send an email to mlir+uns...@tensorflow.org.
To view this discussion on the web visit https://groups.google.com/a/tensorflow.org/d/msgid/mlir/CAPL655inahWNER4fOv_SF16KgVrXeT9MS00Fk0CWQ0WDZvL5iA%40mail.gmail.com.

Mehdi AMINI

unread,
Oct 1, 2019, 1:40:49 PM10/1/19
to Lei Zhang, Alex Zinenko, MLIR
That makes sense to me as well.

The "Ops" name bothered me as well, but not enough to act on it, I'd welcome any patch to clean all this :)

Thanks,

-- 
Mehdi


River Riddle

unread,
Oct 1, 2019, 1:49:46 PM10/1/19
to Mehdi AMINI, Alex Zinenko, Lei Zhang, MLIR

Nicolas Vasilache

unread,
Oct 1, 2019, 1:57:18 PM10/1/19
to MLIR


On Tuesday, October 1, 2019 at 1:40:49 PM UTC-4, Mehdi AMINI wrote:
That makes sense to me as well.

The "Ops" name bothered me as well, but not enough to act on it, I'd welcome any patch to clean all this :)

Level of annoyance is a good forcing function, seems like @Alex reached the threshold on this topic :)
 

Thanks,

-- 
Mehdi


On Tue, Oct 1, 2019 at 7:38 AM 'Lei Zhang' via MLIR <ml...@tensorflow.org> wrote:
+1. I also think it's more consistent to have dialect directory names without "Ops" suffix. But for the source files inside each dialect's directory, it depends on the contents inside. For example, in SPIR-V dialect we have different source files for the dialect itself (SPIRVDialect.cpp), types (SPIRVTypes.cpp), and ops (SPIRVOps.cpp). 

Thanks,
Lei


On Tue, Oct 1, 2019 at 5:01 AM 'Alex Zinenko' via MLIR <ml...@tensorflow.org> wrote:
Hi there,

I noticed that some of the dialects are suffixed with "Ops" and some others are not. Furthermore, the suffix appears only on some parts of the respective file paths, e.g. lib/Dialect/SPIRV/SPIRVOps.cpp vs lib/Dialect/AffineOps/AffineOps.cpp.  It would be great to have a more consistent approach and avoid thinking whether the "Ops" suffix is necessary for every dialect or include file.

My proposition would be to drop the suffix, with the following arguments.
- Dialects contain more than ops (types and attributes as well), some of them actually don't contain any ops and provide attributes for use with other dialects.
- We refer to "standard dialect" or "affine dialect" rather than "standard ops dialect" and "affine ops dialect".
- The suffixing was useful when dialect code was located under top-level lib/ to differentiate between dialects and other libraries; with dialects in lib/Dialect/, it became redundant. 

--
-- Alex

--
You received this message because you are subscribed to the Google Groups "MLIR" group.
To unsubscribe from this group and stop receiving emails from it, send an email to ml...@tensorflow.org.

--
You received this message because you are subscribed to the Google Groups "MLIR" group.
To unsubscribe from this group and stop receiving emails from it, send an email to ml...@tensorflow.org.

Geoffrey Martin-Noble

unread,
Oct 1, 2019, 2:04:43 PM10/1/19
to Nicolas Vasilache, MLIR
+1 to Lei. Remove Ops from the directory name, but keep it in files where all they define is ops.

To unsubscribe from this group and stop receiving emails from it, send an email to mlir+uns...@tensorflow.org.
To view this discussion on the web visit https://groups.google.com/a/tensorflow.org/d/msgid/mlir/29fcfcf7-ae7a-40df-9aff-92633dd17e97%40tensorflow.org.

Alex Zinenko

unread,
Oct 2, 2019, 2:32:02 AM10/2/19
to Mehdi AMINI, Lei Zhang, MLIR

Alex Zinenko

unread,
Oct 2, 2019, 2:33:26 AM10/2/19
to Geoffrey Martin-Noble, Nicolas Vasilache, MLIR
What about the class name for the dialect? Same thing: we have AffineOpsDialect and LLVMDialect for example.

Reply all
Reply to author
Forward
0 new messages