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

[Sparc] Replace CMP instructions with InstAlias (NFCI) #66859

Merged
merged 2 commits into from
Oct 9, 2023

Conversation

s-barannikov
Copy link
Contributor

According to the manual cmp is a synthetic instruction that maps to subcc with %g0 output operand. Make it so.

The change required some changes to instruction selection process. The reason is that the old CMP did not have an output operand, while setcc does have one. We want that operand to be %g0. The easiest way to achieve this seems to be to mark SUBCC with hasPostISelHook and replace the output operand with %g0 in the corresponding TargetLowering method.

@s-barannikov s-barannikov changed the title [Sparc] Replace CMP instructions with InstAlias [Sparc] Replace CMP instructions with InstAlias (NFC) Sep 20, 2023
@llvmbot
Copy link
Member

llvmbot commented Sep 20, 2023

@llvm/pr-subscribers-backend-sparc

Changes

According to the manual cmp is a synthetic instruction that maps to subcc with %g0 output operand. Make it so.

The change required some changes to instruction selection process. The reason is that the old CMP did not have an output operand, while setcc does have one. We want that operand to be %g0. The easiest way to achieve this seems to be to mark SUBCC with hasPostISelHook and replace the output operand with %g0 in the corresponding TargetLowering method.


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

5 Files Affected:

  • (modified) llvm/lib/Target/Sparc/SparcISelLowering.cpp (+8)
  • (modified) llvm/lib/Target/Sparc/SparcISelLowering.h (+4-1)
  • (modified) llvm/lib/Target/Sparc/SparcInstr64Bit.td (+3-2)
  • (modified) llvm/lib/Target/Sparc/SparcInstrAliases.td (+5-2)
  • (modified) llvm/lib/Target/Sparc/SparcInstrInfo.td (+3-12)
diff --git a/llvm/lib/Target/Sparc/SparcISelLowering.cpp b/llvm/lib/Target/Sparc/SparcISelLowering.cpp
index 0091cb7b8bbe7c7..36e19a7fac2f4e0 100644
--- a/llvm/lib/Target/Sparc/SparcISelLowering.cpp
+++ b/llvm/lib/Target/Sparc/SparcISelLowering.cpp
@@ -3644,3 +3644,11 @@ void SparcTargetLowering::insertSSPDeclarations(Module &M) const {
   if (!Subtarget->isTargetLinux())
     return TargetLowering::insertSSPDeclarations(M);
 }
+
+void SparcTargetLowering::AdjustInstrPostInstrSelection(MachineInstr &MI,
+                                                        SDNode *Node) const {
+  assert(MI.getOpcode() == SP::SUBCCrr || MI.getOpcode() == SP::SUBCCri);
+  // If the result is dead, replace it with %g0.
+  if (!Node->hasAnyUseOfValue(0))
+    MI.getOperand(0).setReg(SP::G0);
+}
diff --git a/llvm/lib/Target/Sparc/SparcISelLowering.h b/llvm/lib/Target/Sparc/SparcISelLowering.h
index 5504dcd464fb2d6..9431247a6ef4366 100644
--- a/llvm/lib/Target/Sparc/SparcISelLowering.h
+++ b/llvm/lib/Target/Sparc/SparcISelLowering.h
@@ -223,7 +223,10 @@ namespace llvm {
 
     MachineBasicBlock *expandSelectCC(MachineInstr &MI, MachineBasicBlock *BB,
                                       unsigned BROpcode) const;
+
+    void AdjustInstrPostInstrSelection(MachineInstr &MI,
+                                       SDNode *Node) const override;
   };
 } // end namespace llvm
 
-#endif    // SPARC_ISELLOWERING_H
+#endif // LLVM_LIB_TARGET_SPARC_SPARCISELLOWERING_H
diff --git a/llvm/lib/Target/Sparc/SparcInstr64Bit.td b/llvm/lib/Target/Sparc/SparcInstr64Bit.td
index 189efc32cb2667c..0823c6cf69e91f0 100644
--- a/llvm/lib/Target/Sparc/SparcInstr64Bit.td
+++ b/llvm/lib/Target/Sparc/SparcInstr64Bit.td
@@ -65,6 +65,7 @@ def : Pat<(i64 0), (COPY (i64 G0))>,
   Requires<[Is64Bit]>;
 
 // The ALU instructions want their simm13 operands as i32 immediates.
+// FIXME: This is no longer true, they are now pointer-sized.
 def as_i32imm : SDNodeXForm<imm, [{
   return CurDAG->getTargetConstant(N->getSExtValue(), SDLoc(N), MVT::i32);
 }]>;
@@ -173,8 +174,8 @@ def TLS_ADDXrr : F3_1<2, 0b000000, (outs I64Regs:$rd),
                        (tlsadd i64:$rs1, i64:$rs2, tglobaltlsaddr:$sym))]>;
 }
 
-def : Pat<(SPcmpicc i64:$a, i64:$b), (CMPrr $a, $b)>;
-def : Pat<(SPcmpicc i64:$a, (i64 simm13:$b)), (CMPri $a, (as_i32imm $b))>;
+def : Pat<(SPcmpicc i64:$lhs, i64:$rhs), (SUBCCrr $lhs, $rhs)>;
+def : Pat<(SPcmpicc i64:$lhs, (i64 simm13:$rhs)), (SUBCCri $lhs, imm:$rhs)>;
 def : Pat<(i64 (ctpop i64:$src)), (POPCrr $src)>;
 
 } // Predicates = [Is64Bit]
diff --git a/llvm/lib/Target/Sparc/SparcInstrAliases.td b/llvm/lib/Target/Sparc/SparcInstrAliases.td
index 5d247ac641c7332..db4c05cf180622a 100644
--- a/llvm/lib/Target/Sparc/SparcInstrAliases.td
+++ b/llvm/lib/Target/Sparc/SparcInstrAliases.td
@@ -413,10 +413,13 @@ defm : reg_cond_alias<"gez",  0b111>;
 // non-alias form, except for the most obvious and clarifying aliases: cmp, jmp,
 // call, tst, ret, retl.
 
-// Note: cmp is handled in SparcInstrInfo.
-//       jmp/call/ret/retl have special case handling for output in
+// Note: jmp/call/ret/retl have special case handling for output in
 //       SparcInstPrinter.cpp
 
+// cmp rs1, reg_or_imm -> subcc rs1, reg_or_imm, %g0
+def : InstAlias<"cmp $rs1, $rs2", (SUBCCrr G0, IntRegs:$rs1, IntRegs:$rs2)>;
+def : InstAlias<"cmp $rs1, $imm", (SUBCCri G0, IntRegs:$rs1, simm13Op:$imm)>;
+
 // jmp addr -> jmpl addr, %g0
 def : InstAlias<"jmp $addr", (JMPLrr G0, MEMrr:$addr), 0>;
 def : InstAlias<"jmp $addr", (JMPLri G0, MEMri:$addr), 0>;
diff --git a/llvm/lib/Target/Sparc/SparcInstrInfo.td b/llvm/lib/Target/Sparc/SparcInstrInfo.td
index 3e814643f39e610..5b1695314e9603e 100644
--- a/llvm/lib/Target/Sparc/SparcInstrInfo.td
+++ b/llvm/lib/Target/Sparc/SparcInstrInfo.td
@@ -829,23 +829,14 @@ defm SUB    : F3_12  <"sub"  , 0b000100, sub, IntRegs, i32, simm13Op>;
 let Uses = [ICC], Defs = [ICC] in
   defm SUBE   : F3_12  <"subxcc" , 0b011100, sube, IntRegs, i32, simm13Op>;
 
-let Defs = [ICC] in
+let Defs = [ICC], hasPostISelHook = true in
   defm SUBCC  : F3_12  <"subcc", 0b010100, subc, IntRegs, i32, simm13Op>;
 
 let Uses = [ICC] in
   defm SUBC   : F3_12np <"subx", 0b001100>;
 
-// cmp (from Section A.3) is a specialized alias for subcc
-let Defs = [ICC], rd = 0 in {
-  def CMPrr   : F3_1<2, 0b010100,
-                     (outs), (ins IntRegs:$rs1, IntRegs:$rs2),
-                     "cmp $rs1, $rs2",
-                     [(SPcmpicc i32:$rs1, i32:$rs2)]>;
-  def CMPri   : F3_2<2, 0b010100,
-                     (outs), (ins IntRegs:$rs1, simm13Op:$simm13),
-                     "cmp $rs1, $simm13",
-                     [(SPcmpicc i32:$rs1, (i32 simm13:$simm13))]>;
-}
+def : Pat<(SPcmpicc i32:$lhs, i32:$rhs), (SUBCCri $lhs, $rhs)>;
+def : Pat<(SPcmpicc i32:$lhs, (i32 simm13:$rhs)), (SUBCCri $lhs, imm:$rhs)>;
 
 // Section B.18 - Multiply Instructions, p. 113
 let Defs = [Y] in {

@s-barannikov s-barannikov changed the title [Sparc] Replace CMP instructions with InstAlias (NFC) [Sparc] Replace CMP instructions with InstAlias (NFCI) Sep 20, 2023
@s-barannikov
Copy link
Contributor Author

ping

Copy link
Contributor

@koachan koachan left a comment

Choose a reason for hiding this comment

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

lgtm

According to the manual `cmp` is a synthetic instruction that maps to
`subcc` with %g0 output operand. Make it so.

The change required some changes to instruction selection process. The
reason is that the old CMP did not have an output operand, while setcc
does have one. We want that operand to be %g0. The easiest way to
achieve this seems to be to mark SUBCC with hasPostISelHook and replace
the output operand with %g0 in the corresponding TargetLowering method.
@s-barannikov
Copy link
Contributor Author

Rebased.

@s-barannikov s-barannikov merged commit c98bf1e into llvm:main Oct 9, 2023
2 checks passed
@s-barannikov s-barannikov deleted the sparc-cmp-alias branch October 9, 2023 06:40
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.

4 participants