andykaylor wrote:
> > Can you tell me more about how you intend to use this? This is a very
> > low-level representation. I'd prefer to have less direct coupling with the
> > LLVM IR form in CIR.
>
> Thanks Andy. The use case is lowering AMDGPU builtins. Several of them lower
> to LLVM intrinsics that take a metadata operand, e.g. the `sync-scope` string
> for
> [cooperative_atomic_{load,store}_*](https://github.com/llvm/llvm-project/blob/72b891b75f85/clang/lib/CodeGen/TargetBuiltins/AMDGPU.cpp#L1000-L1011),
>
> [global/flat_load_monitor_*](https://github.com/llvm/llvm-project/blob/72b891b75f85/clang/lib/CodeGen/TargetBuiltins/AMDGPU.cpp#L935-L937)
> etc. CIRGen emits these through `cir.call_llvm_intrinsic`, so it needs a way
> to materialize a metadata value to pass as the operand.
>
> Looking at other targets' CodeGen and CIR's architecture, the dialect is
> designed to be self-contained. It doesn't reference LLVM-dialect specifics
> directly, so it needs its own representation that lowers to them. That's all
> this PR adds: `!cir.metadata` / `#cir.md_string` / `#cir.md_node` /
> `cir.metadata_as_value`, which mirror the LLVM dialect's metadata constructs
> purely so CIR can carry the operand and lower it to the LLVM dialect 1:1.
> They hold no CIR-level semantics; they exist only to feed intrinsic calls.
I'd prefer to see these things modeled at a higher level as much as possible. I
realize that this may become a problem as we get deeper into target-specific
intrinsics, but such intrinsics make the IR somewhat opaque to
target-independent transformations. We already have attributes for expressing
sync-scope on load/store operations. Could you add a `cooperative` attribute to
load and store and defer the intrinsic call creation until lowering to LLVM IR?
There was a similar push years ago in LLVM IR, where we attempted to lower
source-level target intrinsic calls to target-independent representations.
We've drifted from that goal a bit as more and more targets have been added,
but I think target-independent IR should still be the ideal that we strive for.
For cases where extending general operations like load/store isn't practical
but the underlying intrinsic requires a metadata argument (and
global/flat_load_monitor may be such a case), we could introduce
target-specific CIR operations, for example perhaps `cir.amdgcn.load_monitor`?
I just really don't want to introduce an element in CIR that directly maps to
an LLVM IR construct. Tight-coupling with LLVM IR defeats the purpose of MLIR
dialects.
https://github.com/llvm/llvm-project/pull/204190
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
