tpp-mlir icon indicating copy to clipboard operation
tpp-mlir copied to clipboard

Next steps on target descriptor

Open rengolin opened this issue 1 year ago • 2 comments

We have recently merged the target descriptor support in MLIR, and a number of comments were made that were left for follow up PRs.

Below is a list (feel free to add more if I missed anything), in order of what should be done first:

  • [x] use strings everywhere and not have to hard-code integer/float types (signed, etc). llvm/llvm-project/pull/96706
  • [x] Move from specific attributes (ex. L1_cache_size) to string representations like datalayout. Example: cache_hierarchy = 120kb:2mb:30mb; which would just give the whole hierarchy and may be different depending on the target. llvm/llvm-project/pull/101561
  • [x] design the hierarchy of the attributes: module/function/op, multiple/single+lookup, use symbol table/attributes llvm/llvm-project/pull/104595
  • [ ] design a reasonable optional infrastructure where getting a property that is empty or doesn't exist need to fail gracefully.
  • [ ] Design a generic target that is just a pImpl, then passes/transforms use optional + default values (user defined?).
  • [ ] design a hook into LLVM's TTI object, via some target-triple + device + extension strings.
  • [ ] design a hook into reading configuration files (ex. LLVM's Json reader) for custom targets by name.
  • [ ] define the API for adding DLTI information from source (command line options, front-end driver, etc).
  • [ ] design a multi-target hierarchy (ex. CPU+GPU) of descriptors and how to annotate them from other ops.
  • [ ] design a composition of multiple sources for the same target (ex. TLI + Json + cmd-line) that overrides options.

These would make cost models easier and easier, but don't need to finish before we start looking into cost-models.

@rolfmorel

rengolin avatar Jun 21 '24 16:06 rengolin

The current progress on Target Descriptors upstream by extending the DLTI dialect is contained in the following llvm-project PRs:

  1. https://github.com/llvm/llvm-project/pull/92138 ( a continuation of https://github.com/llvm/llvm-project/pull/85141 )
  2. https://github.com/llvm/llvm-project/pull/96706
  3. https://github.com/llvm/llvm-project/pull/101561 ( reland: https://github.com/llvm/llvm-project/pull/102652 )
  4. https://github.com/llvm/llvm-project/pull/104595
  5. https://github.com/llvm/llvm-project/pull/105995
  6. https://github.com/llvm/llvm-project/pull/113365
  7. https://github.com/llvm/llvm-project/pull/115043

Originally, there was the following RFC on Discourse: https://discourse.llvm.org/t/rfc-target-description-and-cost-model-in-mlir/76990

rolfmorel avatar Aug 16 '24 15:08 rolfmorel

@rengolin, I think the following bullet is well-address by [MLIR][DLTI] Introduce DLTIQueryInterface and impl for DLTI attrs - llvm/llvm-project#104595

  • [x] Move from specific attributes (ex. L1_cache_size) to string representations like datalayout. Example: cache_hierarchy = ; which would just give the whole hierarchy and may be different depending on the target.

In particular, you can now represent hierarchical information in a structured way. E.g.,

 #dlti.target_system_spec<"CPU":
   #dlti.target_device_spec<#dlti.dl_entry<"cache",
     #dlti.map<#dlti.dl_entry<"L1",
       #dlti.map<#dlti.dl_entry<"size_in_bytes", 65536 : i32>>,
               #dlti.dl_entry<"L1d",
       #dlti.map<#dlti.dl_entry<"size_in_bytes", 32768 : i32>> >>>>

Note that if someone would prefer to present the information like suggested, #dlti.map<#dlti.dl_entry<"cache_hierachy", "120kb:2mb:30mb">> is a perfectly fine encoding into a DLTI attribute. Though as the contained value is not structured, there would need to be a domain specific interpretation of the string for it to be useful (which is fine if you are just consuming this type of info in your own cost model).

In the context of this example, an alternative to consider is whether to introduce a dlti.cache attribute which has a certain guaranteed structure. E.g., the verifier for this attribute would complain in case the size_in_bytes key is missing in the "L2" sub-dictionary.

rolfmorel avatar Aug 16 '24 15:08 rolfmorel