-
Notifications
You must be signed in to change notification settings - Fork 14.8k
[AArch64][NFC] Use member variable RI instead getRegisterInfo in copyPhysReg #162826
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
[AArch64][NFC] Use member variable RI instead getRegisterInfo in copyPhysReg #162826
Conversation
✅ With the latest revision this PR passed the C/C++ code formatter. |
e5ec4d2
to
3e34b7d
Compare
@llvm/pr-subscribers-backend-aarch64 Author: Tomer Shafir (tomershafir) ChangesThis patch hoists TRI definition on AArch64InstrInfo::copyPhysReg to remove code duplication and improve maintenance. (From performance perspective, as its called for each copy instruction, it can reduce code size, register pressure seems unlikley, and modern processors should trigger this control flow anyway supporting ZCM/ZCZ.) Also, change direct Full diff: https://github.com/llvm/llvm-project/pull/162826.diff 1 Files Affected:
diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
index cad9b1493b86b..c02a78fcaee7e 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
@@ -5066,10 +5066,10 @@ void AArch64InstrInfo::copyPhysReg(MachineBasicBlock &MBB,
Register SrcReg, bool KillSrc,
bool RenamableDest,
bool RenamableSrc) const {
+ const TargetRegisterInfo *TRI = &getRegisterInfo();
+
if (AArch64::GPR32spRegClass.contains(DestReg) &&
(AArch64::GPR32spRegClass.contains(SrcReg) || SrcReg == AArch64::WZR)) {
- const TargetRegisterInfo *TRI = &getRegisterInfo();
-
if (DestReg == AArch64::WSP || SrcReg == AArch64::WSP) {
// If either operand is WSP, expand to ADD #0.
if (Subtarget.hasZeroCycleRegMoveGPR64() &&
@@ -5338,7 +5338,6 @@ void AArch64InstrInfo::copyPhysReg(MachineBasicBlock &MBB,
if (Subtarget.hasZeroCycleRegMoveFPR128() &&
!Subtarget.hasZeroCycleRegMoveFPR64() &&
!Subtarget.hasZeroCycleRegMoveFPR32() && Subtarget.isNeonAvailable()) {
- const TargetRegisterInfo *TRI = &getRegisterInfo();
MCRegister DestRegQ = TRI->getMatchingSuperReg(DestReg, AArch64::dsub,
&AArch64::FPR128RegClass);
MCRegister SrcRegQ = TRI->getMatchingSuperReg(SrcReg, AArch64::dsub,
@@ -5363,7 +5362,6 @@ void AArch64InstrInfo::copyPhysReg(MachineBasicBlock &MBB,
if (Subtarget.hasZeroCycleRegMoveFPR128() &&
!Subtarget.hasZeroCycleRegMoveFPR64() &&
!Subtarget.hasZeroCycleRegMoveFPR32() && Subtarget.isNeonAvailable()) {
- const TargetRegisterInfo *TRI = &getRegisterInfo();
MCRegister DestRegQ = TRI->getMatchingSuperReg(DestReg, AArch64::ssub,
&AArch64::FPR128RegClass);
MCRegister SrcRegQ = TRI->getMatchingSuperReg(SrcReg, AArch64::ssub,
@@ -5378,7 +5376,6 @@ void AArch64InstrInfo::copyPhysReg(MachineBasicBlock &MBB,
.addReg(SrcReg, RegState::Implicit | getKillRegState(KillSrc));
} else if (Subtarget.hasZeroCycleRegMoveFPR64() &&
!Subtarget.hasZeroCycleRegMoveFPR32()) {
- const TargetRegisterInfo *TRI = &getRegisterInfo();
MCRegister DestRegD = TRI->getMatchingSuperReg(DestReg, AArch64::ssub,
&AArch64::FPR64RegClass);
MCRegister SrcRegD = TRI->getMatchingSuperReg(SrcReg, AArch64::ssub,
@@ -5402,7 +5399,6 @@ void AArch64InstrInfo::copyPhysReg(MachineBasicBlock &MBB,
if (Subtarget.hasZeroCycleRegMoveFPR128() &&
!Subtarget.hasZeroCycleRegMoveFPR64() &&
!Subtarget.hasZeroCycleRegMoveFPR32() && Subtarget.isNeonAvailable()) {
- const TargetRegisterInfo *TRI = &getRegisterInfo();
MCRegister DestRegQ = TRI->getMatchingSuperReg(DestReg, AArch64::hsub,
&AArch64::FPR128RegClass);
MCRegister SrcRegQ = TRI->getMatchingSuperReg(SrcReg, AArch64::hsub,
@@ -5417,7 +5413,6 @@ void AArch64InstrInfo::copyPhysReg(MachineBasicBlock &MBB,
.addReg(SrcReg, RegState::Implicit | getKillRegState(KillSrc));
} else if (Subtarget.hasZeroCycleRegMoveFPR64() &&
!Subtarget.hasZeroCycleRegMoveFPR32()) {
- const TargetRegisterInfo *TRI = &getRegisterInfo();
MCRegister DestRegD = TRI->getMatchingSuperReg(DestReg, AArch64::hsub,
&AArch64::FPR64RegClass);
MCRegister SrcRegD = TRI->getMatchingSuperReg(SrcReg, AArch64::hsub,
@@ -5430,10 +5425,10 @@ void AArch64InstrInfo::copyPhysReg(MachineBasicBlock &MBB,
.addReg(SrcRegD, RegState::Undef)
.addReg(SrcReg, RegState::Implicit | getKillRegState(KillSrc));
} else {
- DestReg = RI.getMatchingSuperReg(DestReg, AArch64::hsub,
- &AArch64::FPR32RegClass);
- SrcReg = RI.getMatchingSuperReg(SrcReg, AArch64::hsub,
- &AArch64::FPR32RegClass);
+ DestReg = TRI->getMatchingSuperReg(DestReg, AArch64::hsub,
+ &AArch64::FPR32RegClass);
+ SrcReg = TRI->getMatchingSuperReg(SrcReg, AArch64::hsub,
+ &AArch64::FPR32RegClass);
BuildMI(MBB, I, DL, get(AArch64::FMOVSr), DestReg)
.addReg(SrcReg, getKillRegState(KillSrc));
}
@@ -5445,7 +5440,6 @@ void AArch64InstrInfo::copyPhysReg(MachineBasicBlock &MBB,
if (Subtarget.hasZeroCycleRegMoveFPR128() &&
!Subtarget.hasZeroCycleRegMoveFPR64() &&
!Subtarget.hasZeroCycleRegMoveFPR64() && Subtarget.isNeonAvailable()) {
- const TargetRegisterInfo *TRI = &getRegisterInfo();
MCRegister DestRegQ = TRI->getMatchingSuperReg(DestReg, AArch64::bsub,
&AArch64::FPR128RegClass);
MCRegister SrcRegQ = TRI->getMatchingSuperReg(SrcReg, AArch64::bsub,
@@ -5460,7 +5454,6 @@ void AArch64InstrInfo::copyPhysReg(MachineBasicBlock &MBB,
.addReg(SrcReg, RegState::Implicit | getKillRegState(KillSrc));
} else if (Subtarget.hasZeroCycleRegMoveFPR64() &&
!Subtarget.hasZeroCycleRegMoveFPR32()) {
- const TargetRegisterInfo *TRI = &getRegisterInfo();
MCRegister DestRegD = TRI->getMatchingSuperReg(DestReg, AArch64::bsub,
&AArch64::FPR64RegClass);
MCRegister SrcRegD = TRI->getMatchingSuperReg(SrcReg, AArch64::bsub,
@@ -5473,10 +5466,10 @@ void AArch64InstrInfo::copyPhysReg(MachineBasicBlock &MBB,
.addReg(SrcRegD, RegState::Undef)
.addReg(SrcReg, RegState::Implicit | getKillRegState(KillSrc));
} else {
- DestReg = RI.getMatchingSuperReg(DestReg, AArch64::bsub,
- &AArch64::FPR32RegClass);
- SrcReg = RI.getMatchingSuperReg(SrcReg, AArch64::bsub,
- &AArch64::FPR32RegClass);
+ DestReg = TRI->getMatchingSuperReg(DestReg, AArch64::bsub,
+ &AArch64::FPR32RegClass);
+ SrcReg = TRI->getMatchingSuperReg(SrcReg, AArch64::bsub,
+ &AArch64::FPR32RegClass);
BuildMI(MBB, I, DL, get(AArch64::FMOVSr), DestReg)
.addReg(SrcReg, getKillRegState(KillSrc));
}
@@ -5536,9 +5529,8 @@ void AArch64InstrInfo::copyPhysReg(MachineBasicBlock &MBB,
}
#ifndef NDEBUG
- const TargetRegisterInfo &TRI = getRegisterInfo();
- errs() << TRI.getRegAsmName(DestReg) << " = COPY "
- << TRI.getRegAsmName(SrcReg) << "\n";
+ errs() << TRI->getRegAsmName(DestReg) << " = COPY "
+ << TRI->getRegAsmName(SrcReg) << "\n";
#endif
llvm_unreachable("unimplemented reg-to-reg copy");
}
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think all uses of TRI->
in this function can be replaced with RI.
(which reuses the existing member variable).
…PhysReg This patch uses the RI member variable directly in the member function AArch64InstrInfo::copyPhysReg, instead of redundant calls to the public API.
3e34b7d
to
34e917a
Compare
@MacDue Retargeted the PR to use RI instead |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/72/builds/15444 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/94/builds/11687 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/95/builds/18718 Here is the relevant piece of the build log for the reference
|
…PhysReg (llvm#162826) This patch uses the RI member variable directly in the member function AArch64InstrInfo::copyPhysReg, instead of redundant calls to the public API.
This patch uses the RI member variable directly in the member function AArch64InstrInfo::copyPhysReg, instead of redundant calls to the public API.