Page MenuHomePhabricator

[NVPTX] Refactor generation of MMA intrinsics and instructions. NFC.
ClosedPublic

Authored by tra on Mar 14 2019, 2:50 PM.

Details

Summary

Generalized constructions of 'fragments' of MMA operations to provide common primitives for construction of the ops.
This will make it easier to add new variants of the instructions that operate on integer types.

Use nested foreach loops which makes it possible to better control naming of the intrinsics.

This patch does not affect LLVM's output, so there are no test changes.

Diff Detail

Repository
rL LLVM

Event Timeline

tra created this revision.Mar 14 2019, 2:50 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 14 2019, 2:50 PM
jlebar edited reviewers, added: timshen; removed: jlebar.Mar 14 2019, 4:16 PM
jlebar added a subscriber: jlebar.
timshen added inline comments.Mar 18 2019, 2:40 PM
llvm/include/llvm/IR/IntrinsicsNVVM.td
46 ↗(On Diff #190732)

What's _geom / _frag / _ptx_type? Maybe have some examples, or a comment like one of these down below:
wmma.load.[a|b|c].sync.[row|col].m16n16k16[|.global|.shared].[f16|f32]
or maybe a regex in the comment?

64 ↗(On Diff #190732)

why string llvm? Maybe string intrinsic?

65 ↗(On Diff #190732)

How easy it is to write a string join function? For example:

!join(".", ["llvm", "nvvm", "wmma", "op", frag.frag, Layout] +
    !if(WithStride, [".stride"], []) +
    [frag.ptx_type])

It is slightly more readable and slightly higher level.

Ditto to other string joins.

72 ↗(On Diff #190732)

Usually people use TODO(tra).

74 ↗(On Diff #190732)

!subst(".", "_",
!subst("llvm.", "int_", llvm))?

The difference is that it has a more specific match on "llvm.", instead of "llvm". Only if we can match regex "^llvm\."...

84 ↗(On Diff #190732)

For the parameter names, why is CamelCase mixed with snake_case?

96 ↗(On Diff #190732)

ditto.

llvm/lib/Target/NVPTX/NVPTXIntrinsics.td
7389 ↗(On Diff #190732)

class RegSeq?

7403 ↗(On Diff #190732)

It sounds like _ptx_type can be reworded to ptx_element_type? Also, why the leading underscore?

tra updated this revision to Diff 191201.Mar 18 2019, 4:08 PM
tra marked 11 inline comments as done.
  • Addressed Tim's comments.
llvm/include/llvm/IR/IntrinsicsNVVM.td
65 ↗(On Diff #190732)

I guess we could implement a join, but I don't think it would improve much. Even your example above is rather hard to read. IMO just linear string concat with # is the marginally better as it does not add any further complexity.

84 ↗(On Diff #190732)

Fixed.

llvm/lib/Target/NVPTX/NVPTXIntrinsics.td
7403 ↗(On Diff #190732)

Done.
_ was needed to avoid name clash with a field of WMMA_REGS with the same name.

tra updated this revision to Diff 191202.Mar 18 2019, 4:13 PM
  • Addressed Tim's comments.
timshen accepted this revision.Mar 18 2019, 4:19 PM
This revision is now accepted and ready to land.Mar 18 2019, 4:19 PM
This revision was automatically updated to reflect the committed changes.