Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RISCV][MacroFusion] Improve fusion in pre-RA #82738

Closed
zxc12523 opened this issue Feb 23, 2024 · 4 comments · Fixed by #82751
Closed

[RISCV][MacroFusion] Improve fusion in pre-RA #82738

zxc12523 opened this issue Feb 23, 2024 · 4 comments · Fixed by #82751
Labels
llvm Umbrella label for LLVM issues

Comments

@zxc12523
Copy link
Contributor

#79425 has adopted TableGen-based macro fusion.

And it will generate RISCVGenMacroFusion.inc.

bool isTuneAUIPCADDIFusion(
    const TargetInstrInfo &TII,
    const TargetSubtargetInfo &STI,
    const MachineInstr *FirstMI,
    const MachineInstr &SecondMI) {
  auto &MRI = SecondMI.getMF()->getRegInfo();
  {
    const MachineInstr *MI = &SecondMI;
    if (( MI->getOpcode() != RISCV::ADDI ))
      return false;
  }
  if (!FirstMI)
    return true;
  {
    const MachineInstr *MI = FirstMI;
    if (( MI->getOpcode() != RISCV::AUIPC ))
      return false;
  }
  {
    const MachineInstr *MI = &SecondMI;
    if (!(
        MI->getOperand(0).getReg().isVirtual()
        || MI->getOperand(0).getReg() == MI->getOperand(1).getReg()
      ))
      return false;
  }
  {
    Register FirstDest = FirstMI->getOperand(0).getReg();
    if (FirstDest.isVirtual() && !MRI.hasOneNonDBGUse(FirstDest))
      return false;
  }
  if (!(FirstMI->getOperand(0).isReg() &&
        SecondMI.getOperand(1).isReg() &&
        FirstMI->getOperand(0).getReg() == SecondMI.getOperand(1).getReg()))
    return false;
  return true;
}

My question is, is it possible to check FirstMI->getOperand(0).getReg() == SecondMI.getOperand(2).getReg() when these two regs are virtual and the second inst is commutative?

@zxc12523 zxc12523 changed the title [RISCV][MACROFusion] Improve fusion in pre-RA [RISCV][MacroFusion] Improve fusion in pre-RA Feb 23, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 23, 2024

@llvm/issue-subscribers-backend-risc-v

Author: None (zxc12523)

#79425 has adopted TableGen-based macro fusion.

And it will generate RISCVGenMacroFusion.inc.

bool isTuneAUIPCADDIFusion(
    const TargetInstrInfo &TII,
    const TargetSubtargetInfo &STI,
    const MachineInstr *FirstMI,
    const MachineInstr &SecondMI) {
  auto &MRI = SecondMI.getMF()->getRegInfo();
  {
    const MachineInstr *MI = &SecondMI;
    if (( MI->getOpcode() != RISCV::ADDI ))
      return false;
  }
  if (!FirstMI)
    return true;
  {
    const MachineInstr *MI = FirstMI;
    if (( MI->getOpcode() != RISCV::AUIPC ))
      return false;
  }
  {
    const MachineInstr *MI = &SecondMI;
    if (!(
        MI->getOperand(0).getReg().isVirtual()
        || MI->getOperand(0).getReg() == MI->getOperand(1).getReg()
      ))
      return false;
  }
  {
    Register FirstDest = FirstMI->getOperand(0).getReg();
    if (FirstDest.isVirtual() && !MRI.hasOneNonDBGUse(FirstDest))
      return false;
  }
  if (!(FirstMI->getOperand(0).isReg() &&
        SecondMI.getOperand(1).isReg() &&
        FirstMI->getOperand(0).getReg() == SecondMI.getOperand(1).getReg()))
    return false;
  return true;
}

My question is, is it possible to check FirstMI->getOperand(0).getReg() == SecondMI.getOperand(2).getReg() when these two regs are virtual and the second inst is commutative?

@wangpc-pp
Copy link
Contributor

is it possible to check FirstMI->getOperand(0).getReg() == SecondMI.getOperand(2).getReg() when these two regs are virtual and the second inst is commutative?

Yes, it can be set via adding a TieReg<0, 2>.

This can be done automatically, I will create a PR for this. Thanks for reporting!

wangpc-pp added a commit to wangpc-pp/llvm-project that referenced this issue Feb 23, 2024
If the second instruction is commutable, we should be able to check
its commutable operands.

A simple RISCV fusion is contained in this PR to show the functionality
is correct, I may remove it when landing.

There are some other issues I should fix. For example, we should be
able to check that the destination register is tha same as the
commutable operands. But I post this PR here firstly to gather
feedbacks.

Fixes llvm#82738
@zxc12523
Copy link
Contributor Author

Thanks for your response! Are there any tablegen learning materials or source code reading tools I can use? I am a newbie and I want to learn more about this topic.

@wangpc-pp
Copy link
Contributor

wangpc-pp commented Feb 26, 2024

Thanks for your response! Are there any tablegen learning materials or source code reading tools I can use? I am a newbie and I want to learn more about this topic.

You mean the TableGen-based macro fusion? I have some links that you can refer to:

Some code links:

  • // The target instruction that FusionPredicate will be evaluated on.
    class FusionTarget;
    def first_fusion_target : FusionTarget;
    def second_fusion_target : FusionTarget;
    def both_fusion_target : FusionTarget;
    // Base class of FusionPredicate, etc. The avaliable variables are:
    // * const TargetInstrInfo &TII
    // * const TargetSubtargetInfo &STI
    // * const MachineRegisterInfo &MRI
    // * const MachineInstr *FirstMI
    // * const MachineInstr &SecondMI
    class FusionPredicate<FusionTarget target> {
    FusionTarget Target = target;
    }
    class FirstFusionPredicate: FusionPredicate<first_fusion_target>;
    class SecondFusionPredicate: FusionPredicate<second_fusion_target>;
    class BothFusionPredicate: FusionPredicate<both_fusion_target>;
    // FusionPredicate with raw code predicate.
    class FusionPredicateWithCode<code pred> : FusionPredicate<both_fusion_target> {
    code Predicate = pred;
    }
    // FusionPredicate with MCInstPredicate.
    class FusionPredicateWithMCInstPredicate<FusionTarget target, MCInstPredicate pred>
    : FusionPredicate<target> {
    MCInstPredicate Predicate = pred;
    }
    class FirstFusionPredicateWithMCInstPredicate<MCInstPredicate pred>
    : FusionPredicateWithMCInstPredicate<first_fusion_target, pred>;
    class SecondFusionPredicateWithMCInstPredicate<MCInstPredicate pred>
    : FusionPredicateWithMCInstPredicate<second_fusion_target, pred>;
    // The pred will be applied on both firstMI and secondMI.
    class BothFusionPredicateWithMCInstPredicate<MCInstPredicate pred>
    : FusionPredicateWithMCInstPredicate<second_fusion_target, pred>;
    // Tie firstOpIdx and secondOpIdx. The operand of `FirstMI` at position
    // `firstOpIdx` should be the same as the operand of `SecondMI` at position
    // `secondOpIdx`.
    class TieReg<int firstOpIdx, int secondOpIdx> : BothFusionPredicate {
    int FirstOpIdx = firstOpIdx;
    int SecondOpIdx = secondOpIdx;
    }
    // A predicate for wildcard. The generated code will be like:
    // ```
    // if (!FirstMI)
    // return ReturnValue;
    // ```
    class WildcardPred<bit ret> : FirstFusionPredicate {
    bit ReturnValue = ret;
    }
    def WildcardFalse : WildcardPred<0>;
    def WildcardTrue : WildcardPred<1>;
    // Indicates that the destination register of `FirstMI` should have one use if
    // it is a virtual register.
    class OneUsePred : FirstFusionPredicate;
    def OneUse : OneUsePred;
    // Handled by MacroFusionPredicatorEmitter backend.
    // The generated predicator will be like:
    // ```
    // bool isNAME(const TargetInstrInfo &TII,
    // const TargetSubtargetInfo &STI,
    // const MachineInstr *FirstMI,
    // const MachineInstr &SecondMI) {
    // auto &MRI = SecondMI.getMF()->getRegInfo();
    // /* Predicates */
    // return true;
    // }
    // ```
    class Fusion<string name, string fieldName, string desc, list<FusionPredicate> predicates>
    : SubtargetFeature<name, fieldName, "true", desc> {
    list<FusionPredicate> Predicates = predicates;
    }
    // The generated predicator will be like:
    // ```
    // bool isNAME(const TargetInstrInfo &TII,
    // const TargetSubtargetInfo &STI,
    // const MachineInstr *FirstMI,
    // const MachineInstr &SecondMI) {
    // auto &MRI = SecondMI.getMF()->getRegInfo();
    // /* Prolog */
    // /* Predicate for `SecondMI` */
    // /* Wildcard */
    // /* Predicate for `FirstMI` */
    // /* Check One Use */
    // /* Tie registers */
    // /* Epilog */
    // return true;
    // }
    // ```
    class SimpleFusion<string name, string fieldName, string desc,
    MCInstPredicate firstPred, MCInstPredicate secondPred,
    list<FusionPredicate> prolog = [],
    list<FusionPredicate> epilog = []>
    : Fusion<name, fieldName, desc,
    !listconcat(
    prolog,
    [
    SecondFusionPredicateWithMCInstPredicate<secondPred>,
    WildcardTrue,
    FirstFusionPredicateWithMCInstPredicate<firstPred>,
    SecondFusionPredicateWithMCInstPredicate<
    CheckAny<[
    CheckIsVRegOperand<0>,
    CheckSameRegOperand<0, 1>
    ]>>,
    OneUse,
    TieReg<0, 1>,
    ],
    epilog)>;
    These are the TableGen definitions. I think they are expressive but not so easy to use (I'm continuing to do some improvements).
  • https://github.com/llvm/llvm-project/blob/main/llvm/utils/TableGen/MacroFusionPredicatorEmitter.cpp. This is the TableGen backend that generates these predicators.

wangpc-pp added a commit to wangpc-pp/llvm-project that referenced this issue Feb 27, 2024
If the second instruction is commutable, we should be able to check
its commutable operands.

A simple RISCV fusion is contained in this PR to show the functionality
is correct, I may remove it when landing.

There are some other issues I should fix. For example, we should be
able to check that the destination register is tha same as the
commutable operands. But I post this PR here firstly to gather
feedbacks.

Fixes llvm#82738
wangpc-pp added a commit to wangpc-pp/llvm-project that referenced this issue Mar 5, 2024
If the second instruction is commutable, we should be able to check
its commutable operands.

A simple RISCV fusion is contained in this PR to show the functionality
is correct, I may remove it when landing.

Fixes llvm#82738
wangpc-pp added a commit to wangpc-pp/llvm-project that referenced this issue Mar 14, 2024
If the second instruction is commutable, we should be able to check
its commutable operands.

A field `IsCommutable` is added to indicate whether we should generate
code for checking commutable operands.

A simple RISCV fusion is contained in this PR to show the functionality
is correct, I may remove it when landing.

Fixes llvm#82738
wangpc-pp added a commit to wangpc-pp/llvm-project that referenced this issue Mar 15, 2024
If the second instruction is commutable, we should be able to check
its commutable operands.

A field `IsCommutable` is added to indicate whether we should generate
code for checking commutable operands.

Fixes llvm#82738
wangpc-pp added a commit that referenced this issue Mar 15, 2024
If the second instruction is commutable, we should be able to check
its commutable operands.

A simple RISCV fusion is contained in this PR to show the functionality
is correct, I may remove it when landing.

Fixes #82738
@EugeneZelenko EugeneZelenko added llvm Umbrella label for LLVM issues and removed backend:RISC-V labels Mar 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm Umbrella label for LLVM issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants