[PATCH] D90853: [RISCV] Add DAG nodes to represent read/write CSR

[Serge Pavlov via Phabricator via llvm-commits]

sepavloff added a comment.

In D90853#2489631 <https://reviews.llvm.org/D90853#2489631>, @rogfer01 wrote:

> Hi Serge,
>
>> Using `X0` as output is just a trick to have a new instruction without spending opcode. Actually such instruction does not define `X0`. What is the benefit of exposing this low-level encoding feature in high-level structures?
>
> My suggestion was to avoid the situation where we have two machine instructions that overlap in their semantics. This entails that a later pass that analyses CSRs should take into account those write only forms in addition to the actual instructions. However, maybe this is not a practical issue. The number of CSR instructions is not large. It may also happen that SelectionDAG will never select a CSR write instruction that writes to `X0`. Or if it does, we would always use the new write-only form that you suggest.

The new instruction patterns are marked as `isCodegenOnly`, so they won't appear in MC layer. For example, output of disassembler may not contain them. Any analysis made at this level may be unaware of the new patterns. It is only codegen that must take into account the new pattern. But for it these instructions indeed are different, they have different fundamental properties.

It would be, of course, convenient to have close correspondence between instruction bit representation and instruction object used in internal representation. It is however sometimes not possible and codegens often use  `isCodegenOnly` patterns to cope with gap between encoding and machine IR representation. For example X86 defines different patterns for XOR8 depending on whether it is prefixed with REX prefix. Any analysis made at MachineInstr level need to take into account all variants. And X86 is not an exception.

> I'm not sure if we would want to prefix those write-only versions with `Pseudo`? (like it happens with other instructions that exist for the purpose of Codegen).

Defining `isCodegenOnly` pattern is a clear and compact solution, `Pseudo` would require additional code to expand it. Besides it is not clear what could be a replacement in this case. It can't be CSRRW with X0 as defined register.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90853