From cbaa7877bdb4447d345d3e9e7ea6bc1df7a56b3e Mon Sep 17 00:00:00 2001 From: DianQK Date: Tue, 12 Dec 2023 08:00:12 +0800 Subject: [PATCH 1/7] Pre-commit test case --- .../CodeGen/AArch64/machine-cp-sub-reg.mir | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 llvm/test/CodeGen/AArch64/machine-cp-sub-reg.mir diff --git a/llvm/test/CodeGen/AArch64/machine-cp-sub-reg.mir b/llvm/test/CodeGen/AArch64/machine-cp-sub-reg.mir new file mode 100644 index 0000000000000..7fdc94ec510e9 --- /dev/null +++ b/llvm/test/CodeGen/AArch64/machine-cp-sub-reg.mir @@ -0,0 +1,32 @@ +# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 4 +# RUN: llc -o - %s -O3 --run-pass=machine-cp -mcp-use-is-copy-instr -mtriple=arm64-apple-macos -mcpu=apple-m1 --verify-machineinstrs | FileCheck %s + +--- +name: test +tracksRegLiveness: true +body: | + ; CHECK-LABEL: name: test + ; CHECK: bb.0: + ; CHECK-NEXT: successors: %bb.1(0x80000000) + ; CHECK-NEXT: liveins: $w0 + ; CHECK-NEXT: {{ $}} + ; CHECK-NEXT: $x8 = ORRXrs $xzr, $x0, 0, implicit $w0 + ; CHECK-NEXT: {{ $}} + ; CHECK-NEXT: bb.1: + ; CHECK-NEXT: liveins: $x8 + ; CHECK-NEXT: {{ $}} + ; CHECK-NEXT: $x0 = ADDXri $x8, 1, 0 + ; CHECK-NEXT: RET undef $lr, implicit $x0 + bb.0: + successors: %bb.1(0x80000000) + liveins: $w0 + + $x8 = ORRXrs $xzr, undef $x0, 0, implicit $w0 + $w8 = ORRWrs $wzr, $w0, 0, implicit-def $x8 + + bb.1: + liveins: $x8 + $x0 = ADDXri $x8, 1, 0 + + RET undef $lr, implicit $x0 +... From 8dfe4229580dcf113163b0c06a2758ad28181624 Mon Sep 17 00:00:00 2001 From: DianQK Date: Tue, 12 Dec 2023 19:02:13 +0800 Subject: [PATCH 2/7] [AArch64] ORRWrs is copy instruction when there's no implicit def of the X register --- llvm/lib/Target/AArch64/AArch64InstrInfo.cpp | 25 +++++++++++++------ .../CodeGen/AArch64/machine-cp-sub-reg.mir | 3 ++- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp index 50cbd3672fbd0..0f6aeff26fdd4 100644 --- a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp +++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp @@ -9180,18 +9180,29 @@ void AArch64InstrInfo::buildClearRegister(Register Reg, MachineBasicBlock &MBB, std::optional AArch64InstrInfo::isCopyInstrImpl(const MachineInstr &MI) const { - // AArch64::ORRWrs and AArch64::ORRXrs with WZR/XZR reg // and zero immediate operands used as an alias for mov instruction. - if (MI.getOpcode() == AArch64::ORRWrs && - MI.getOperand(1).getReg() == AArch64::WZR && - MI.getOperand(3).getImm() == 0x0) { + bool OpIsORRWrs = MI.getOpcode() == AArch64::ORRWrs; + bool OpIsORRXrs = MI.getOpcode() == AArch64::ORRXrs; + if (!(OpIsORRWrs || OpIsORRXrs) || MI.getOperand(3).getImm() != 0x0) + return std::nullopt; + Register Reg1 = MI.getOperand(1).getReg(); + + if (OpIsORRWrs && Reg1 == AArch64::WZR) { + Register Reg0 = MI.getOperand(0).getReg(); + if (Reg0.isPhysical()) { + const MachineFunction *MF = MI.getMF(); + const TargetRegisterInfo *TRI = MF->getSubtarget().getRegisterInfo(); + for (const MachineOperand &MO : MI.implicit_operands()) + if (MO.isDef() && MO.isImplicit() && + TRI->isSubRegister(MO.getReg(), Reg0)) { + return std::nullopt; + } + } return DestSourcePair{MI.getOperand(0), MI.getOperand(2)}; } - if (MI.getOpcode() == AArch64::ORRXrs && - MI.getOperand(1).getReg() == AArch64::XZR && - MI.getOperand(3).getImm() == 0x0) { + if (OpIsORRXrs && Reg1 == AArch64::XZR) { return DestSourcePair{MI.getOperand(0), MI.getOperand(2)}; } diff --git a/llvm/test/CodeGen/AArch64/machine-cp-sub-reg.mir b/llvm/test/CodeGen/AArch64/machine-cp-sub-reg.mir index 7fdc94ec510e9..6c09f2ce7fcc1 100644 --- a/llvm/test/CodeGen/AArch64/machine-cp-sub-reg.mir +++ b/llvm/test/CodeGen/AArch64/machine-cp-sub-reg.mir @@ -10,7 +10,8 @@ body: | ; CHECK-NEXT: successors: %bb.1(0x80000000) ; CHECK-NEXT: liveins: $w0 ; CHECK-NEXT: {{ $}} - ; CHECK-NEXT: $x8 = ORRXrs $xzr, $x0, 0, implicit $w0 + ; CHECK-NEXT: $x8 = ORRXrs $xzr, undef $x0, 0, implicit $w0 + ; CHECK-NEXT: $w8 = ORRWrs $wzr, $w0, 0, implicit-def $x8 ; CHECK-NEXT: {{ $}} ; CHECK-NEXT: bb.1: ; CHECK-NEXT: liveins: $x8 From 7f10332cc295dccec156ab649eb53c580718772b Mon Sep 17 00:00:00 2001 From: DianQK Date: Wed, 13 Dec 2023 22:45:45 +0800 Subject: [PATCH 3/7] [TargetInstrInfo] Add `isCopyLikeInstr` method --- llvm/include/llvm/CodeGen/TargetInstrInfo.h | 13 +++++++++++++ .../CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp | 2 +- .../lib/CodeGen/LiveDebugValues/VarLocBasedImpl.cpp | 4 ++-- llvm/lib/Target/AArch64/AArch64InstrInfo.cpp | 13 ++++++++++++- llvm/lib/Target/AArch64/AArch64InstrInfo.h | 2 ++ 5 files changed, 30 insertions(+), 4 deletions(-) diff --git a/llvm/include/llvm/CodeGen/TargetInstrInfo.h b/llvm/include/llvm/CodeGen/TargetInstrInfo.h index 2bbe430dc68d9..a113100f04e62 100644 --- a/llvm/include/llvm/CodeGen/TargetInstrInfo.h +++ b/llvm/include/llvm/CodeGen/TargetInstrInfo.h @@ -1025,6 +1025,11 @@ class TargetInstrInfo : public MCInstrInfo { return std::nullopt; } + virtual std::optional + isCopyLikeInstrImpl(const MachineInstr &MI) const { + return std::nullopt; + } + /// Return true if the given terminator MI is not expected to spill. This /// sets the live interval as not spillable and adjusts phi node lowering to /// not introduce copies after the terminator. Use with care, these are @@ -1050,6 +1055,14 @@ class TargetInstrInfo : public MCInstrInfo { return isCopyInstrImpl(MI); } + // Similar to `isCopyInstr`, but adds non-copy semantics on MIR, but + // ultimately generates a copy instruction. + std::optional isCopyLikeInstr(const MachineInstr &MI) const { + if (auto IsCopyInstr = isCopyInstr(MI)) + return IsCopyInstr; + return isCopyLikeInstrImpl(MI); + } + bool isFullCopyInstr(const MachineInstr &MI) const { auto DestSrc = isCopyInstr(MI); if (!DestSrc) diff --git a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp index b2c2b40139eda..8a78a51f72ebc 100644 --- a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp +++ b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp @@ -2116,7 +2116,7 @@ bool InstrRefBasedLDV::transferSpillOrRestoreInst(MachineInstr &MI) { } bool InstrRefBasedLDV::transferRegisterCopy(MachineInstr &MI) { - auto DestSrc = TII->isCopyInstr(MI); + auto DestSrc = TII->isCopyLikeInstr(MI); if (!DestSrc) return false; diff --git a/llvm/lib/CodeGen/LiveDebugValues/VarLocBasedImpl.cpp b/llvm/lib/CodeGen/LiveDebugValues/VarLocBasedImpl.cpp index 116c6b7e2d19e..bf730be00a9a9 100644 --- a/llvm/lib/CodeGen/LiveDebugValues/VarLocBasedImpl.cpp +++ b/llvm/lib/CodeGen/LiveDebugValues/VarLocBasedImpl.cpp @@ -1364,7 +1364,7 @@ void VarLocBasedLDV::removeEntryValue(const MachineInstr &MI, // TODO: Try to keep tracking of an entry value if we encounter a propagated // DBG_VALUE describing the copy of the entry value. (Propagated entry value // does not indicate the parameter modification.) - auto DestSrc = TII->isCopyInstr(*TransferInst); + auto DestSrc = TII->isCopyLikeInstr(*TransferInst); if (DestSrc) { const MachineOperand *SrcRegOp, *DestRegOp; SrcRegOp = DestSrc->Source; @@ -1840,7 +1840,7 @@ void VarLocBasedLDV::transferRegisterCopy(MachineInstr &MI, OpenRangesSet &OpenRanges, VarLocMap &VarLocIDs, TransferMap &Transfers) { - auto DestSrc = TII->isCopyInstr(MI); + auto DestSrc = TII->isCopyLikeInstr(MI); if (!DestSrc) return; diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp index 0f6aeff26fdd4..ebae0d42ff68c 100644 --- a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp +++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp @@ -9190,6 +9190,8 @@ AArch64InstrInfo::isCopyInstrImpl(const MachineInstr &MI) const { if (OpIsORRWrs && Reg1 == AArch64::WZR) { Register Reg0 = MI.getOperand(0).getReg(); + // ORRWrs is copy instruction when there's no implicit def of the X + // register. if (Reg0.isPhysical()) { const MachineFunction *MF = MI.getMF(); const TargetRegisterInfo *TRI = MF->getSubtarget().getRegisterInfo(); @@ -9209,6 +9211,15 @@ AArch64InstrInfo::isCopyInstrImpl(const MachineInstr &MI) const { return std::nullopt; } +std::optional +AArch64InstrInfo::isCopyLikeInstrImpl(const MachineInstr &MI) const { + if (MI.getOpcode() == AArch64::ORRWrs && + MI.getOperand(1).getReg() == AArch64::WZR && + MI.getOperand(3).getImm() == 0x0) + return DestSourcePair{MI.getOperand(0), MI.getOperand(2)}; + return std::nullopt; +} + std::optional AArch64InstrInfo::isAddImmediate(const MachineInstr &MI, Register Reg) const { int Sign = 1; @@ -9252,7 +9263,7 @@ static std::optional describeORRLoadedValue(const MachineInstr &MI, Register DescribedReg, const TargetInstrInfo *TII, const TargetRegisterInfo *TRI) { - auto DestSrc = TII->isCopyInstr(MI); + auto DestSrc = TII->isCopyLikeInstr(MI); if (!DestSrc) return std::nullopt; diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.h b/llvm/lib/Target/AArch64/AArch64InstrInfo.h index e97ff0a9758d6..6526f6740747a 100644 --- a/llvm/lib/Target/AArch64/AArch64InstrInfo.h +++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.h @@ -401,6 +401,8 @@ class AArch64InstrInfo final : public AArch64GenInstrInfo { /// registers as machine operands. std::optional isCopyInstrImpl(const MachineInstr &MI) const override; + std::optional + isCopyLikeInstrImpl(const MachineInstr &MI) const override; private: unsigned getInstBundleLength(const MachineInstr &MI) const; From 80aa729430a9cfcd3e93b4bee40c2c84535b380b Mon Sep 17 00:00:00 2001 From: DianQK Date: Wed, 13 Dec 2023 23:08:39 +0800 Subject: [PATCH 4/7] Remove undef --- llvm/test/CodeGen/AArch64/machine-cp-sub-reg.mir | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/llvm/test/CodeGen/AArch64/machine-cp-sub-reg.mir b/llvm/test/CodeGen/AArch64/machine-cp-sub-reg.mir index 6c09f2ce7fcc1..f3d1bb1f5f324 100644 --- a/llvm/test/CodeGen/AArch64/machine-cp-sub-reg.mir +++ b/llvm/test/CodeGen/AArch64/machine-cp-sub-reg.mir @@ -10,7 +10,7 @@ body: | ; CHECK-NEXT: successors: %bb.1(0x80000000) ; CHECK-NEXT: liveins: $w0 ; CHECK-NEXT: {{ $}} - ; CHECK-NEXT: $x8 = ORRXrs $xzr, undef $x0, 0, implicit $w0 + ; CHECK-NEXT: $x8 = ORRXrs $xzr, $x0, 0, implicit $w0 ; CHECK-NEXT: $w8 = ORRWrs $wzr, $w0, 0, implicit-def $x8 ; CHECK-NEXT: {{ $}} ; CHECK-NEXT: bb.1: @@ -22,7 +22,7 @@ body: | successors: %bb.1(0x80000000) liveins: $w0 - $x8 = ORRXrs $xzr, undef $x0, 0, implicit $w0 + $x8 = ORRXrs $xzr, $x0, 0, implicit $w0 $w8 = ORRWrs $wzr, $w0, 0, implicit-def $x8 bb.1: From fc94442ba9d308fee540b396ebf3418a8f0c729e Mon Sep 17 00:00:00 2001 From: DianQK Date: Thu, 14 Dec 2023 07:16:14 +0800 Subject: [PATCH 5/7] Remove -O3 and -mcpu --- llvm/test/CodeGen/AArch64/machine-cp-sub-reg.mir | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/test/CodeGen/AArch64/machine-cp-sub-reg.mir b/llvm/test/CodeGen/AArch64/machine-cp-sub-reg.mir index f3d1bb1f5f324..23cf1dcda839e 100644 --- a/llvm/test/CodeGen/AArch64/machine-cp-sub-reg.mir +++ b/llvm/test/CodeGen/AArch64/machine-cp-sub-reg.mir @@ -1,5 +1,5 @@ # NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 4 -# RUN: llc -o - %s -O3 --run-pass=machine-cp -mcp-use-is-copy-instr -mtriple=arm64-apple-macos -mcpu=apple-m1 --verify-machineinstrs | FileCheck %s +# RUN: llc -o - %s --run-pass=machine-cp -mcp-use-is-copy-instr -mtriple=arm64-apple-macos --verify-machineinstrs | FileCheck %s --- name: test From 736ba3ad470c7f04f35b30d7473c00d2e028ede6 Mon Sep 17 00:00:00 2001 From: DianQK Date: Thu, 14 Dec 2023 07:21:24 +0800 Subject: [PATCH 6/7] Update only the ORRWrs branch --- llvm/lib/Target/AArch64/AArch64InstrInfo.cpp | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp index ebae0d42ff68c..4f2360cd3796d 100644 --- a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp +++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp @@ -9180,15 +9180,12 @@ void AArch64InstrInfo::buildClearRegister(Register Reg, MachineBasicBlock &MBB, std::optional AArch64InstrInfo::isCopyInstrImpl(const MachineInstr &MI) const { + // AArch64::ORRWrs and AArch64::ORRXrs with WZR/XZR reg // and zero immediate operands used as an alias for mov instruction. - bool OpIsORRWrs = MI.getOpcode() == AArch64::ORRWrs; - bool OpIsORRXrs = MI.getOpcode() == AArch64::ORRXrs; - if (!(OpIsORRWrs || OpIsORRXrs) || MI.getOperand(3).getImm() != 0x0) - return std::nullopt; - Register Reg1 = MI.getOperand(1).getReg(); - - if (OpIsORRWrs && Reg1 == AArch64::WZR) { + if (MI.getOpcode() == AArch64::ORRWrs && + MI.getOperand(1).getReg() == AArch64::WZR && + MI.getOperand(3).getImm() == 0x0) { Register Reg0 = MI.getOperand(0).getReg(); // ORRWrs is copy instruction when there's no implicit def of the X // register. @@ -9204,9 +9201,10 @@ AArch64InstrInfo::isCopyInstrImpl(const MachineInstr &MI) const { return DestSourcePair{MI.getOperand(0), MI.getOperand(2)}; } - if (OpIsORRXrs && Reg1 == AArch64::XZR) { + if (MI.getOpcode() == AArch64::ORRXrs && + MI.getOperand(1).getReg() == AArch64::XZR && + MI.getOperand(3).getImm() == 0x0) return DestSourcePair{MI.getOperand(0), MI.getOperand(2)}; - } return std::nullopt; } From f245648358410c0c9c382e91011edc0bf20944cc Mon Sep 17 00:00:00 2001 From: DianQK Date: Thu, 14 Dec 2023 07:28:09 +0800 Subject: [PATCH 7/7] Use findRegisterDefOperandIdx --- llvm/lib/Target/AArch64/AArch64InstrInfo.cpp | 21 +++++++------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp index 4f2360cd3796d..7d71c316bcb0a 100644 --- a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp +++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp @@ -9185,21 +9185,14 @@ AArch64InstrInfo::isCopyInstrImpl(const MachineInstr &MI) const { // and zero immediate operands used as an alias for mov instruction. if (MI.getOpcode() == AArch64::ORRWrs && MI.getOperand(1).getReg() == AArch64::WZR && - MI.getOperand(3).getImm() == 0x0) { - Register Reg0 = MI.getOperand(0).getReg(); - // ORRWrs is copy instruction when there's no implicit def of the X - // register. - if (Reg0.isPhysical()) { - const MachineFunction *MF = MI.getMF(); - const TargetRegisterInfo *TRI = MF->getSubtarget().getRegisterInfo(); - for (const MachineOperand &MO : MI.implicit_operands()) - if (MO.isDef() && MO.isImplicit() && - TRI->isSubRegister(MO.getReg(), Reg0)) { - return std::nullopt; - } - } + MI.getOperand(3).getImm() == 0x0 && + // Check that the w->w move is not a zero-extending w->x mov. + (!MI.getOperand(0).getReg().isVirtual() || + MI.getOperand(0).getSubReg() == 0) && + (!MI.getOperand(0).getReg().isPhysical() || + MI.findRegisterDefOperandIdx(MI.getOperand(0).getReg() - AArch64::W0 + + AArch64::X0) == -1)) return DestSourcePair{MI.getOperand(0), MI.getOperand(2)}; - } if (MI.getOpcode() == AArch64::ORRXrs && MI.getOperand(1).getReg() == AArch64::XZR &&