[PowerPC] Adjust operand order of ADDItoc to be consistent with other ADDI* nodes #93642
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-backend-powerpc Author: Kai Luo (bzEq) ChangesFull diff: https://github.com/llvm/llvm-project/pull/93642.diff 6 Files Affected:
diff --git a/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp b/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
index f4e84ade3b5ac..bc0ae7a32c051 100644
--- a/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
+++ b/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
@@ -1079,13 +1079,13 @@ void PPCAsmPrinter::emitInstruction(const MachineInstr *MI) {
assert(IsAIX && TM.getCodeModel() == CodeModel::Small &&
"PseudoOp only valid for small code model AIX");
- // Transform %rN = ADDItoc/8 @op1, %r2.
+ // Transform %rN = ADDItoc/8 %r2, @op1.
LowerPPCMachineInstrToMCInst(MI, TmpInst, *this);
// Change the opcode to load address.
TmpInst.setOpcode((!IsPPC64) ? (PPC::LA) : (PPC::LA8));
- const MachineOperand &MO = MI->getOperand(1);
+ const MachineOperand &MO = MI->getOperand(2);
assert(MO.isGlobal() && "Invalid operand for ADDItoc[8].");
// Map the operand to its corresponding MCSymbol.
@@ -1094,7 +1094,6 @@ void PPCAsmPrinter::emitInstruction(const MachineInstr *MI) {
const MCExpr *Exp =
MCSymbolRefExpr::create(MOSymbol, MCSymbolRefExpr::VK_None, OutContext);
- TmpInst.getOperand(1) = TmpInst.getOperand(2);
TmpInst.getOperand(2) = MCOperand::createExpr(Exp);
EmitToStreamer(*OutStreamer, TmpInst);
return;
diff --git a/llvm/lib/Target/PowerPC/PPCFastISel.cpp b/llvm/lib/Target/PowerPC/PPCFastISel.cpp
index 735050641adff..a07954bd0d8b3 100644
--- a/llvm/lib/Target/PowerPC/PPCFastISel.cpp
+++ b/llvm/lib/Target/PowerPC/PPCFastISel.cpp
@@ -2080,13 +2080,15 @@ unsigned PPCFastISel::PPCMaterializeGV(const GlobalValue *GV, MVT VT) {
cast<GlobalVariable>(GV)->hasAttribute("toc-data");
// For small code model, generate a simple TOC load.
- if (CModel == CodeModel::Small)
- BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD,
- IsAIXTocData ? TII.get(PPC::ADDItoc8) : TII.get(PPC::LDtoc),
- DestReg)
- .addGlobalAddress(GV)
- .addReg(PPC::X2);
- else {
+ if (CModel == CodeModel::Small) {
+ auto MIB = BuildMI(
+ *FuncInfo.MBB, FuncInfo.InsertPt, MIMD,
+ IsAIXTocData ? TII.get(PPC::ADDItoc8) : TII.get(PPC::LDtoc), DestReg);
+ if (IsAIXTocData)
+ MIB.addReg(PPC::X2).addGlobalAddress(GV);
+ else
+ MIB.addGlobalAddress(GV).addReg(PPC::X2);
+ } else {
// If the address is an externally defined symbol, a symbol with common
// or externally available linkage, a non-local function address, or a
// jump table address (not yet needed), or if we are generating code
diff --git a/llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp b/llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp
index 26560dc5cdebb..9c1f5f5e18e31 100644
--- a/llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp
+++ b/llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp
@@ -6096,7 +6096,11 @@ void PPCDAGToDAGISel::Select(SDNode *N) {
EVT OperandTy) {
SDValue GA = TocEntry->getOperand(0);
SDValue TocBase = TocEntry->getOperand(1);
- SDNode *MN = CurDAG->getMachineNode(OpCode, dl, OperandTy, GA, TocBase);
+ SDNode *MN = nullptr;
+ if (OpCode == PPC::ADDItoc || OpCode == PPC::ADDItoc8)
+ MN = CurDAG->getMachineNode(OpCode, dl, OperandTy, TocBase, GA);
+ else
+ MN = CurDAG->getMachineNode(OpCode, dl, OperandTy, GA, TocBase);
transferMemOperands(TocEntry, MN);
ReplaceNode(TocEntry, MN);
};
diff --git a/llvm/lib/Target/PowerPC/PPCInstr64Bit.td b/llvm/lib/Target/PowerPC/PPCInstr64Bit.td
index 9af8ada783761..d3760b76969f9 100644
--- a/llvm/lib/Target/PowerPC/PPCInstr64Bit.td
+++ b/llvm/lib/Target/PowerPC/PPCInstr64Bit.td
@@ -1485,10 +1485,10 @@ def ADDItocL8: PPCEmitTimePseudo<(outs g8rc:$rD), (ins g8rc_nox0:$reg, tocentry:
}
// Local Data Transform
-def ADDItoc8 : PPCEmitTimePseudo<(outs g8rc:$rD), (ins tocentry:$disp, g8rc_nox0:$reg),
+def ADDItoc8 : PPCEmitTimePseudo<(outs g8rc:$rD), (ins g8rc_nox0:$reg, tocentry:$disp),
"#ADDItoc8",
[(set i64:$rD,
- (PPCtoc_entry tglobaladdr:$disp, i64:$reg))]>, isPPC64;
+ (PPCtoc_entry i64:$reg, tglobaladdr:$disp))]>, isPPC64;
let mayLoad = 1 in
def LDtocL: PPCEmitTimePseudo<(outs g8rc:$rD), (ins tocentry:$disp, g8rc_nox0:$reg),
diff --git a/llvm/lib/Target/PowerPC/PPCInstrInfo.td b/llvm/lib/Target/PowerPC/PPCInstrInfo.td
index e3d6d2f094f2e..5f6fff7e9d4f8 100644
--- a/llvm/lib/Target/PowerPC/PPCInstrInfo.td
+++ b/llvm/lib/Target/PowerPC/PPCInstrInfo.td
@@ -3347,10 +3347,10 @@ def ADDIStocHA : PPCEmitTimePseudo<(outs gprc:$rD), (ins gprc_nor0:$reg, tocentr
[(set i32:$rD,
(PPCtoc_entry i32:$reg, tglobaladdr:$disp))]>;
// TOC Data Transform on AIX
-def ADDItoc : PPCEmitTimePseudo<(outs gprc:$rD), (ins tocentry32:$disp, gprc:$reg),
+def ADDItoc : PPCEmitTimePseudo<(outs gprc:$rD), (ins gprc:$reg, tocentry32:$disp),
"#ADDItoc",
[(set i32:$rD,
- (PPCtoc_entry tglobaladdr:$disp, i32:$reg))]>;
+ (PPCtoc_entry i32:$reg, tglobaladdr:$disp))]>;
def ADDItocL : PPCEmitTimePseudo<(outs gprc:$rD), (ins gprc_nor0:$reg, tocentry32:$disp),
"#ADDItocL", []>;
diff --git a/llvm/test/CodeGen/PowerPC/toc-data.ll b/llvm/test/CodeGen/PowerPC/toc-data.ll
index 12286657488db..ee1dde190bb26 100644
--- a/llvm/test/CodeGen/PowerPC/toc-data.ll
+++ b/llvm/test/CodeGen/PowerPC/toc-data.ll
@@ -36,7 +36,7 @@ define dso_local void @write_int(i32 signext %in) {
ret void
}
; CHECK32: name: write_int
-; CHECK32: %[[SCRATCH:[0-9]+]]:gprc_and_gprc_nor0 = ADDItoc @i, $r2
+; CHECK32: %[[SCRATCH:[0-9]+]]:gprc_and_gprc_nor0 = ADDItoc $r2, @i
; CHECK32-NEXT: STW %{{[0-9]+}}, 0, killed %[[SCRATCH]] :: (store (s32) into @i)
; TEST32: .write_int:
@@ -44,12 +44,12 @@ define dso_local void @write_int(i32 signext %in) {
; TEST32-NEXT: stw 3, 0(4)
; CHECK64: name: write_int
-; CHECK64: %[[SCRATCH:[0-9]+]]:g8rc_and_g8rc_nox0 = ADDItoc8 @i, $x2
+; CHECK64: %[[SCRATCH:[0-9]+]]:g8rc_and_g8rc_nox0 = ADDItoc8 $x2, @i
; CHECK64-NEXT: STW8 %{{[0-9]+}}, 0, killed %[[SCRATCH]] :: (store (s32) into @i)
; CHECK64-NOOPT: name: write_int
; CHECK64-NOOPT: %[[SUBREG:[0-9]+]]:gprc = COPY %{{[0-9]}}.sub_32
-; CHECK64-NOOPT: %[[ADDR:[0-9]+]]:g8rc_and_g8rc_nox0 = ADDItoc8 @i, $x2
+; CHECK64-NOOPT: %[[ADDR:[0-9]+]]:g8rc_and_g8rc_nox0 = ADDItoc8 $x2, @i
; CHECK64-NOOPT: STW %[[SUBREG]], 0, %[[ADDR]]
; TEST64: .write_int:
@@ -128,7 +128,7 @@ define dso_local float @read_float() {
ret float %0
}
; CHECK32: name: read_float
-; CHECK32: %[[SCRATCH:[0-9]+]]:gprc_and_gprc_nor0 = ADDItoc @f, $r2
+; CHECK32: %[[SCRATCH:[0-9]+]]:gprc_and_gprc_nor0 = ADDItoc $r2, @f
; CHECK32: %{{[0-9]+}}:f4rc = LFS 0, killed %[[SCRATCH]] :: (dereferenceable load (s32) from @f)
; TEST32: .read_float:
@@ -136,11 +136,11 @@ define dso_local float @read_float() {
; TEST32-NEXT: lfs 1, 0(3)
; CHECK64: name: read_float
-; CHECK64: %[[SCRATCH:[0-9]+]]:g8rc_and_g8rc_nox0 = ADDItoc8 @f, $x2
+; CHECK64: %[[SCRATCH:[0-9]+]]:g8rc_and_g8rc_nox0 = ADDItoc8 $x2, @f
; CHECK64: %{{[0-9]+}}:f4rc = LFS 0, killed %[[SCRATCH]] :: (dereferenceable load (s32) from @f)
; CHECK64-NOOPT: name: read_float
-; CHECK64-NOOPT: %[[SCRATCH:[0-9]+]]:g8rc_and_g8rc_nox0 = ADDItoc8 @f, $x2
+; CHECK64-NOOPT: %[[SCRATCH:[0-9]+]]:g8rc_and_g8rc_nox0 = ADDItoc8 $x2, @f
; CHECK64-NOOPT: %{{[0-9]+}}:f4rc = LFS 0, %[[SCRATCH]]
; TEST64: .read_float:
@@ -217,18 +217,18 @@ define dso_local nonnull ptr @addr() {
ret ptr @i
}
; CHECK32: name: addr
-; CHECK32: %[[SCRATCH:[0-9]+]]:gprc = ADDItoc @i, $r2
+; CHECK32: %[[SCRATCH:[0-9]+]]:gprc = ADDItoc $r2, @i
; CHECK32-NEXT: $r3 = COPY %[[SCRATCH]]
; TEST32: .addr
; TEST32: la 3, i[TD](2)
; CHECK64: name: addr
-; CHECK64: %[[SCRATCH:[0-9]+]]:g8rc = ADDItoc8 @i, $x2
+; CHECK64: %[[SCRATCH:[0-9]+]]:g8rc = ADDItoc8 $x2, @i
; CHECK64-NEXT: $x3 = COPY %[[SCRATCH]]
; CHECK64-NOOPT: name: addr
-; CHECK64-NOOPT: %[[SCRATCH:[0-9]+]]:g8rc_and_g8rc_nox0 = ADDItoc8 @i, $x2
+; CHECK64-NOOPT: %[[SCRATCH:[0-9]+]]:g8rc_and_g8rc_nox0 = ADDItoc8 $x2, @i
; CHECK64-NOOPT: $x3 = COPY %[[SCRATCH]]
; TEST64: .addr
|
mandlebug
reviewed
May 30, 2024
| "#ADDItoc8", | ||
| [(set i64:$rD, | ||
| (PPCtoc_entry tglobaladdr:$disp, i64:$reg))]>, isPPC64; | ||
| (PPCtoc_entry i64:$reg, tglobaladdr:$disp))]>, isPPC64; |
There was a problem hiding this comment.
Why are we changing this pattern (ditto for the ADDItoc pattern as well)? If this was meant to be a match pattern for a PPCtoc_entry node I belive the operands are always a target global address followed by the TOC-base register. here and here
I don't think we ever rely on pattern matching for transforming to these nodes though (since we need to manually check for the attribute on the Global Symbol), so in that case we are best to remove the pattern.
There was a problem hiding this comment.
That's really a good point.
bzEq
changed the title
|
After removing the pattern, I don't think it's an NFC now, since memoperands are removed from |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Simultaneously, the
ADDItocmachineinstr is generated inPPCISelDAGToDAG::Selectso the pattern is not used and can be removed.