[PATCH] D54777: [AArch64] Refactor the scheduling predicates (NFC)

[Andrea Di Biagio via Phabricator via llvm-commits]

andreadb added a comment.

Hi Evandro.

Thanks for the patch. However, it is quite big to review.

Can you split it into multiple patches?

I think you can start by just rewiting method `isScaledAddr`.
It would be easier for me (and other reviewers) to see the differences in the auto-generated .inc file.

About `isScaledAddr`:
You can use a MCOpcodeSwitchStatement and rewrite that definition to something like this:

  // Common variants to various AArch64 processor models.
  
  let FunctionMapper = "AArch64_AM::getMemExtendType" in
  def CheckExtendType : CheckImmOperand_s<3, "AArch64_AM::UXTX">;
  
  let FunctionMapper = "AArch64_AM::getMemDoShift" in
  def CheckFoldedShift : CheckImmOperandSimple<3>;
  
  // ScaledIdxPred is true if a WriteLDIdx operand will be
  // scaled. Subtargets can use this to dynamically select resources and
  // latency for WriteLDIdx and ReadAdrBase.
  //
  // Method 'isScaledAddr' Returns true if this is load/store scales or extends
  // its register offset. This refers to scaling a dynamic index as opposed to
  // scaled immediates.  MI should be a memory op that allows scaled addressing.
  def ScaledIdxPred : TIIPredicate<"isScaledAddr",
    MCOpcodeSwitchStatement<[
      MCOpcodeSwitchCase<[
          LDRBBroW, LDRBroW, LDRDroW,
          LDRHHroW, LDRHroW, LDRQroW,
          LDRSBWroW, LDRSBXroW, LDRSHWroW, LDRSHXroW,
          LDRSWroW, LDRSroW, LDRWroW, LDRXroW,
  
          STRBBroW, STRBroW, STRDroW,
          STRHHroW, STRHroW, STRQroW,
          STRSroW, STRWroW, STRXroW,
  
          LDRBBroX, LDRBroX, LDRDroX,
          LDRHHroX, LDRHroX, LDRQroX,
          LDRSBWroX, LDRSBXroX, LDRSHWroX, LDRSHXroX,
          LDRSWroX, LDRSroX, LDRWroX, LDRXroX,
  
          STRBBroX, STRBroX, STRDroX,
          STRHHroX, STRHroX, STRQroX,
          STRSroX, STRWroX, STRXroX
        ],
        MCReturnStatement<CheckAny<[
          CheckNot<CheckExtendType>,
          CheckFoldedShift ]>>
      >], MCReturnStatement<FalsePred>
    >
  >;

The resulting code expanded by tablegen should be more readable (opcodes appear as cases of a switch-stmt).

I sugges that you to split this NFC patch into multiple patches.
To start, you can send a patch that rewrites `isScaledAddr` only, and then verify that the auto-generated definition in the aarch64 gen-instrinfo .inc file is still semantically correct.
You should be able to add llvm-mca tests to verify that writes which require `isScaledAddr` to resolve their scheduling class are now correctly analyzed by llvm-mca.

-Andrea


Repository:
  rL LLVM

https://reviews.llvm.org/D54777