[all-commits] [llvm/llvm-project] d04d64: [RISCV] Add SchedRead for merge operand

Michael Maitland via All-commits all-commits at lists.llvm.org
Sun Aug 13 12:30:55 PDT 2023


  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: d04d645f0eee10d9fe0ccdff9bd83212fb9dfc78
      https://github.com/llvm/llvm-project/commit/d04d645f0eee10d9fe0ccdff9bd83212fb9dfc78
  Author: Michael Maitland <michaeltmaitland at gmail.com>
  Date:   2023-08-13 (Sun, 13 Aug 2023)

  Changed paths:
    M llvm/lib/Target/RISCV/RISCVInstrInfoV.td
    M llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td

  Log Message:
  -----------
  [RISCV] Add SchedRead for merge operand

Prior to this patch, SchedReads were not modeled for instructions that
did not have a MASK suffix. This patch adds a SchedRead for all
instructions that have a merge operand.

@reames is doing work to fold TA and TU instructions into a single
instruction. This means that a single instruction may or may not
actually read the merge operand (TU must read merge operand). It is
possible that a TA instruction needs to read the merge operand, for
instance if TA is implemented as TU. Therefore, it makes sense to
represent the read of the merge operand for both TA and TU instructions.

In the case that a subtarget wants to model TA read different from TU
read, the subtarget should use a SchedVariant, which has access to the
MachineInstr.

Without this patch, the current SchedReads are off by one for
instructions that have a merge operand.

I am concerned is that `forceMergeOpRead` is passed to `SchedXXX`, but
the `SchedXXX` is not defined next to the ins and outs. This leads us to
walk the class hierarchy to determine whether `forceMergeOpRead` needs to be
be true. I worry that this will make this change harder to review, and
it may not be clear that `forceMergeOpRead` needs to be updated if any
future changes add or remove merge operands. I thought about moving the SchedXXX
definitions to where the ins and outs occur (as a seperate patch), but the
drawback of this is that we have to pass around lots of new arguments through
class heirarchies. One improvement on this is to use a single custom data
structure to pass through the heirarchies the eventually gets used by the
SchedXXX. If any reviewers care to share their own opinion on this concern, or
suggest a different solution to this concern, it would be greatly appreciated.
In any case, this concern is NFC.

Differential Revision: https://reviews.llvm.org/D157650




More information about the All-commits mailing list