This is an archive of the discontinued LLVM Phabricator instance.

[mlir][SPIRV] Update dialect/op documentation to be consistent with the rest of the dialects
AbandonedPublic

Authored by rriddle on Mar 24 2020, 2:59 PM.

Details

Summary

This revision performs a lot of different cleanups on operation documentation to ensure that they are consistent:

  • using mlir code blocks
  • formatting
  • removing custom syntax for operations that have it auto-generated

Diff Detail

Event Timeline

rriddle created this revision.Mar 24 2020, 2:59 PM
mravishankar requested changes to this revision.Mar 25 2020, 3:35 PM

A lot of this is auto-generated using https://github.com/llvm/llvm-project/blob/master/mlir/utils/spirv/gen_spirv_dialect.py . Would be better to update that so that it doesnt get reverted when we use the script to regenerate the .td files. Let me know if you want one of us to take that up (should be a simple change.

This revision now requires changes to proceed.Mar 25 2020, 3:35 PM

A lot of this is auto-generated using https://github.com/llvm/llvm-project/blob/master/mlir/utils/spirv/gen_spirv_dialect.py . Would be better to update that so that it doesnt get reverted when we use the script to regenerate the .td files. Let me know if you want one of us to take that up (should be a simple change.

Yes that would be great. Is that going to subsume all of the changes here in the .td files? Note, the .md changes are still necessary regardless.

Actually, strike what I said earlier. The script uses the existing text for an op.

But can you just update these two lines of the script to match what you have here. https://github.com/llvm/llvm-project/blob/92744f624783d92a07db25bc76e181b879f17e5b/mlir/utils/spirv/gen_spirv_dialect.py#L677 - #L678

Also I think (@antiagainst can confirm), the ### Custom assembly format is used as a demarcartor by the generator script. I tried to find cases above where it is removed.

mlir/include/mlir/Dialect/SPIRV/SPIRVLogicalOps.td
825

Please dont remove this. It is used by the dialect generator script as a demarcator

mlir/include/mlir/Dialect/SPIRV/SPIRVNonUniformOps.td
70

Please dont remove this. It is used by the dialect generator script as a demarcator

119

Please dont remove this. It is used by the dialect generator script as a demarcator

mlir/include/mlir/Dialect/SPIRV/SPIRVOps.td
137

Please dont remove this. It is used by the dialect generator script as a demarcator

299

Please dont remove this. It is used by the dialect generator script as a demarcator

394

Please dont remove this. It is used by the dialect generator script as a demarcator

mlir/include/mlir/Dialect/SPIRV/SPIRVStructureOps.td
36

Please dont remove this. It is used by the dialect generator script as a demarcator

480

Please dont remove this. It is used by the dialect generator script as a demarcator

The first part of description (before ### Custom assembly form) is automatically populated from the SPIR-V spec by the gen_spirv_dialect.py tool. The part after ### Custom assembly form is manually added. So ### Custom assembly form is the anchor at the moment to split. To remove it means we need rethink how the documentation is done. (The Example ... mlir part is fine as Mahesh pointed out.) Can I ask what do we want to achieve here? Will you automatically generate the custom assembly form's doc some way?

The first part of description (before ### Custom assembly form) is automatically populated from the SPIR-V spec by the gen_spirv_dialect.py tool. The part after ### Custom assembly form is manually added. So ### Custom assembly form is the anchor at the moment to split. To remove it means we need rethink how the documentation is done. (The Example ... mlir part is fine as Mahesh pointed out.) Can I ask what do we want to achieve here? Will you automatically generate the custom assembly form's doc some way?

The overall goal is to bring some uniformity to the dialect documentation, and clean it up. The assembly form weirdness comes from the fact that we already generate a section for the syntax when the operation uses the assemblyFormat field, which leads to some redundancy: e.g. https://mlir.llvm.org/docs/Dialects/SPIRVDialect/#spv_address_of-spirvaddressofop

The first part of description (before ### Custom assembly form) is automatically populated from the SPIR-V spec by the gen_spirv_dialect.py tool. The part after ### Custom assembly form is manually added. So ### Custom assembly form is the anchor at the moment to split. To remove it means we need rethink how the documentation is done. (The Example ... mlir part is fine as Mahesh pointed out.) Can I ask what do we want to achieve here? Will you automatically generate the custom assembly form's doc some way?

The overall goal is to bring some uniformity to the dialect documentation, and clean it up. The assembly form weirdness comes from the fact that we already generate a section for the syntax when the operation uses the assemblyFormat field, which leads to some redundancy: e.g. https://mlir.llvm.org/docs/Dialects/SPIRVDialect/#spv_address_of-spirvaddressofop

Great. I see now. Let me take over and fix the script to use a different anchor. Thanks River!

rriddle abandoned this revision.Apr 8 2020, 2:49 PM