[PATCH] D98936: [RISCV] DAG nodes and pseudo instructions for CSR access

Wed Mar 31 07:57:06 PDT 2021

sepavloff added a comment.

In D98936#2661518 <https://reviews.llvm.org/D98936#2661518>, @jrtc27 wrote:

> In D98936#2661504 <https://reviews.llvm.org/D98936#2661504>, @sepavloff wrote:
>
>> In D98936#2661232 <https://reviews.llvm.org/D98936#2661232>, @jrtc27 wrote:
>>
>>> In D98936#2661228 <https://reviews.llvm.org/D98936#2661228>, @asb wrote:
>>>
>>>> In D98936#2642039 <https://reviews.llvm.org/D98936#2642039>, @jrtc27 wrote:
>>>>
>>>>> Are there ever any cases where you _wouldn't_ want a CSR-specific pseudo in order to have control over the scheduling of it specifically? This feels a bit like a middle-ground that's the worst of both worlds to me.
>>>>
>>>> Hi @jrtc27 - could you please elaborate a little on the concern that the approach in this path might be the worst of both worlds? I'm not sure I fully follow. Thanks.
>>>
>>> If you want to have different scheduling for different CSRs, do you not need per-CSR pseudos in order to express that? This diff is ostensibly to allow for that in future, but is at a much coarser read/write/swap granularity, so doesn't really get you much over and above just scheduling CSRRW itself as a whole, just adds more complexity for little gain. IMO any CSRs we need scheduling info for should just get their own dedicated read/write/swap pseudos as and when they're needed.
>>
>> The patch D99083 <https://reviews.llvm.org/D99083> demonstrates the solution for FP state/control resisters. Every register and every access get separate pseudos, each of which can have their own scheduling properties.
>
> That looks like the kind of thing I'm imagining. So can we remove the Read/Write/Swap_CSR defs from this diff and just keep the Read/Write/SwapSysReg classes for use with such pseudos?

No problem. I just thought that putting Read/Write/Swap_CSR defs (which are general purpose definitions) into a patch specific for FP support is not quite logical. But if this is OK, I'll move them.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98936/new/

https://reviews.llvm.org/D98936