[Mlir-commits] [flang] [mlir] [MLIR][LLVMIR][DLTI] Add #llvm.target, #llvm.data_layout and TargetAttrInterface (PR #145899)

[Rolf Morel]

rolfmorel wrote:

Couple of pain points in need of suggestions/feedback:

1. As `LLVMTargetFeaturesAttr` exists we have the following:
   1. There's a circular dependency between `LLVMInterfaces` and `LLVMAttrs` in that the new `TargetAttrInterface` requires `LLVMTargetFeaturesAttr` (for the `features` of a target) and there are attrs in `LLVMAttrs` that require interfaces from `LLVMInterfaces`. I break the dependency by moving `LLVMTargetFeaturesAttr` to its own file and having a dedicated CMake-rule to generate `LLVMOpsAttrDefs.h.inc` by way of `LLVMAttrAndEnumDefs.td`.
        1. Is there a better way of dealing with this circular dependency? 
   3. Currently, `#nvvm.target` and `#rocdl.target` take `features` as a `StringAttr`. Seems to me they should take `LLVMTargetFeaturesAttr` which would make them eligible for/compatible with implementing `TargetAttrInterface`. I have not spent time on transitioning them yet.
4.  For some reason, the parser generated for ``let assemblyFormat = [{`<` `triple` `=` $triple (`,` `chip` `=` $chip^)? (`,` qualified($features)^)? `>`}];`` does not actually make the `chip` clause optional. That is, it greedily assumes that as it saw `,` from that clause and it is now committed to it. For example, `#llvm.target<triple = "x86_64-unknown-linux", #llvm.target_features<["+mmx", "+sse"]>>` yields an `expected 'chip'` parser error.
5. How do I properly hang a `unique_ptr<llvm::TargetMachine>` of off `TargetAttr`? The current bare pointer works but probably is not safe, e.g. in case the context gets deleted. See discussion with @rengolin above.
6. Had to move the `DataLayoutImporter` from `lib/Target/LLVMIR` to `Dialect/LLVMIR/IR`, in support of `#llvm.data_layout` being a wrapper around the corresponding `#dlti.dl_spec`, which causes a linking dependency on the DLTI-dialect.
7. What is the strategy around verification? See discussion with @rengolin above.
8. Test cases will need to be made aware of which backends the compiled `llvm-project` has available and be skipped if not applicable. 

https://github.com/llvm/llvm-project/pull/145899