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] Remove FrameIndex case in lui+addi MacroFusion #68701

Merged
merged 1 commit into from
Oct 19, 2023

Conversation

wangpc-pp
Copy link
Contributor

Correct me if I am wrong, if the first operand of ADDI is a frame
index, then it won't have data dependency of predecessor LUI. So
it is impossible to do the DAG mutation in these two instructions.

Correct me if I am wrong, if the first operand of ADDI is a frame
index, then it won't have data dependency of predecessor LUI. So
it is impossible to do the DAG mutation in these two instructions.
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 10, 2023

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

Author: Wang Pengcheng (wangpc-pp)

Changes

Correct me if I am wrong, if the first operand of ADDI is a frame
index, then it won't have data dependency of predecessor LUI. So
it is impossible to do the DAG mutation in these two instructions.


Full diff: https://github.com/llvm/llvm-project/pull/68701.diff

1 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVMacroFusion.cpp (-4)
diff --git a/llvm/lib/Target/RISCV/RISCVMacroFusion.cpp b/llvm/lib/Target/RISCV/RISCVMacroFusion.cpp
index da104657680a6b9..02a8d5c18fe1a0e 100644
--- a/llvm/lib/Target/RISCV/RISCVMacroFusion.cpp
+++ b/llvm/lib/Target/RISCV/RISCVMacroFusion.cpp
@@ -35,10 +35,6 @@ static bool isLUIADDI(const MachineInstr *FirstMI,
   if (FirstMI->getOpcode() != RISCV::LUI)
     return false;
 
-  // The first operand of ADDI might be a frame index.
-  if (!SecondMI.getOperand(1).isReg())
-    return false;
-
   Register FirstDest = FirstMI->getOperand(0).getReg();
 
   // Destination of LUI should be the ADDI(W) source register.

@wangpc-pp
Copy link
Contributor Author

Ping.

Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants