[llvm] Remove SDNPSideEffect from ARMcallseq_start and ARMcallseq_end (NFC) (PR #153248)

[via llvm-commits]

https://github.com/AZero13 updated https://github.com/llvm/llvm-project/pull/153248

>From 55de8b20d24917e2ade631bf117030c3f8340d92 Mon Sep 17 00:00:00 2001
From: AZero13 <gfunni234 at gmail.com>
Date: Tue, 12 Aug 2025 14:35:31 -0400
Subject: [PATCH 1/4] [ARM] Simplify ADJCALLSTACKUP and ADJCALLSTACKDOWN on ARM

A call sequence does not have any unmodeled side effects in of itself.

ADJCALLSTACKUP and ADJCALLSTACKDOWN do, however, so the attribute should be there.

Finally, make ADJCALLSTACKUP and ADJCALLSTACKDOWN codegen only.
---
 llvm/lib/Target/ARM/ARMInstrInfo.td  | 11 +++--------
 llvm/lib/Target/ARM/ARMInstrThumb.td |  5 +----
 2 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/llvm/lib/Target/ARM/ARMInstrInfo.td b/llvm/lib/Target/ARM/ARMInstrInfo.td
index 934ec52c6f1e4..a3bf32446695e 100644
--- a/llvm/lib/Target/ARM/ARMInstrInfo.td
+++ b/llvm/lib/Target/ARM/ARMInstrInfo.td
@@ -164,10 +164,9 @@ def ARMWrapperPIC    : SDNode<"ARMISD::WrapperPIC",  SDTIntUnaryOp>;
 def ARMWrapperJT     : SDNode<"ARMISD::WrapperJT",   SDTIntUnaryOp>;
 
 def ARMcallseq_start : SDNode<"ISD::CALLSEQ_START", SDT_ARMCallSeqStart,
-                              [SDNPHasChain, SDNPSideEffect, SDNPOutGlue]>;
+                              [SDNPHasChain, SDNPOutGlue]>;
 def ARMcallseq_end   : SDNode<"ISD::CALLSEQ_END",   SDT_ARMCallSeqEnd,
-                              [SDNPHasChain, SDNPSideEffect,
-                               SDNPOptInGlue, SDNPOutGlue]>;
+                              [SDNPHasChain, SDNPOptInGlue, SDNPOutGlue]>;
 def ARMcopystructbyval : SDNode<"ARMISD::COPY_STRUCT_BYVAL" ,
                                 SDT_ARMStructByVal,
                                 [SDNPHasChain, SDNPInGlue, SDNPOutGlue,
@@ -2203,11 +2202,7 @@ def JUMPTABLE_TBH :
 PseudoInst<(outs), (ins cpinst_operand:$instid, cpinst_operand:$cpidx,
                         i32imm:$size), NoItinerary, []>;
 
-
-// FIXME: Marking these as hasSideEffects is necessary to prevent machine DCE
-// from removing one half of the matched pairs. That breaks PEI, which assumes
-// these will always be in pairs, and asserts if it finds otherwise. Better way?
-let Defs = [SP], Uses = [SP], hasSideEffects = 1 in {
+let Defs = [SP], Uses = [SP], hasSideEffects = 1, isCodeGenOnly = 1 in {
 def ADJCALLSTACKUP :
 PseudoInst<(outs), (ins i32imm:$amt1, i32imm:$amt2, pred:$p), NoItinerary,
            [(ARMcallseq_end timm:$amt1, timm:$amt2)]>;
diff --git a/llvm/lib/Target/ARM/ARMInstrThumb.td b/llvm/lib/Target/ARM/ARMInstrThumb.td
index e38cafdf55c46..3d23874e117aa 100644
--- a/llvm/lib/Target/ARM/ARMInstrThumb.td
+++ b/llvm/lib/Target/ARM/ARMInstrThumb.td
@@ -297,10 +297,7 @@ def non_imm32 : PatLeaf<(i32 GPR), [{ return !isa<ConstantSDNode>(N); }]>;
 //  Miscellaneous Instructions.
 //
 
-// FIXME: Marking these as hasSideEffects is necessary to prevent machine DCE
-// from removing one half of the matched pairs. That breaks PEI, which assumes
-// these will always be in pairs, and asserts if it finds otherwise. Better way?
-let Defs = [SP], Uses = [SP], hasSideEffects = 1 in {
+let Defs = [SP], Uses = [SP], hasSideEffects = 1, isCodeGenOnly = 1 in {
 def tADJCALLSTACKUP :
   PseudoInst<(outs), (ins i32imm:$amt1, i32imm:$amt2), NoItinerary,
              [(ARMcallseq_end imm:$amt1, imm:$amt2)]>,

>From e0d797a81b6862f1aea5c1b499a83c6de7390ffa Mon Sep 17 00:00:00 2001
From: AZero13 <gfunni234 at gmail.com>
Date: Thu, 14 Aug 2025 14:05:21 -0400
Subject: [PATCH 2/4] Remove SDNPSideEffect from ARMcallseq_start and
 ARMcallseq_end

ADJCALLSTACKUP and ADJCALLSTACKDOWN has the side effecs, not the sequence in itself.
---
 llvm/lib/Target/ARM/ARMInstrInfo.td  | 2 +-
 llvm/lib/Target/ARM/ARMInstrThumb.td | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/llvm/lib/Target/ARM/ARMInstrInfo.td b/llvm/lib/Target/ARM/ARMInstrInfo.td
index a3bf32446695e..a54672c460476 100644
--- a/llvm/lib/Target/ARM/ARMInstrInfo.td
+++ b/llvm/lib/Target/ARM/ARMInstrInfo.td
@@ -2202,7 +2202,7 @@ def JUMPTABLE_TBH :
 PseudoInst<(outs), (ins cpinst_operand:$instid, cpinst_operand:$cpidx,
                         i32imm:$size), NoItinerary, []>;
 
-let Defs = [SP], Uses = [SP], hasSideEffects = 1, isCodeGenOnly = 1 in {
+let Defs = [SP], Uses = [SP], hasSideEffects = 1 in {
 def ADJCALLSTACKUP :
 PseudoInst<(outs), (ins i32imm:$amt1, i32imm:$amt2, pred:$p), NoItinerary,
            [(ARMcallseq_end timm:$amt1, timm:$amt2)]>;
diff --git a/llvm/lib/Target/ARM/ARMInstrThumb.td b/llvm/lib/Target/ARM/ARMInstrThumb.td
index 3d23874e117aa..483a432690c96 100644
--- a/llvm/lib/Target/ARM/ARMInstrThumb.td
+++ b/llvm/lib/Target/ARM/ARMInstrThumb.td
@@ -297,7 +297,7 @@ def non_imm32 : PatLeaf<(i32 GPR), [{ return !isa<ConstantSDNode>(N); }]>;
 //  Miscellaneous Instructions.
 //
 
-let Defs = [SP], Uses = [SP], hasSideEffects = 1, isCodeGenOnly = 1 in {
+let Defs = [SP], Uses = [SP], hasSideEffects = 1 in {
 def tADJCALLSTACKUP :
   PseudoInst<(outs), (ins i32imm:$amt1, i32imm:$amt2), NoItinerary,
              [(ARMcallseq_end imm:$amt1, imm:$amt2)]>,

>From 99958a68ff8e2e460ef67001b38bcba963b6feaa Mon Sep 17 00:00:00 2001
From: AZero13 <gfunni234 at gmail.com>
Date: Mon, 18 Aug 2025 11:44:25 -0400
Subject: [PATCH 3/4] Put comment back

---
 llvm/lib/Target/ARM/ARMInstrThumb.td | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/llvm/lib/Target/ARM/ARMInstrThumb.td b/llvm/lib/Target/ARM/ARMInstrThumb.td
index 483a432690c96..e38cafdf55c46 100644
--- a/llvm/lib/Target/ARM/ARMInstrThumb.td
+++ b/llvm/lib/Target/ARM/ARMInstrThumb.td
@@ -297,6 +297,9 @@ def non_imm32 : PatLeaf<(i32 GPR), [{ return !isa<ConstantSDNode>(N); }]>;
 //  Miscellaneous Instructions.
 //
 
+// FIXME: Marking these as hasSideEffects is necessary to prevent machine DCE
+// from removing one half of the matched pairs. That breaks PEI, which assumes
+// these will always be in pairs, and asserts if it finds otherwise. Better way?
 let Defs = [SP], Uses = [SP], hasSideEffects = 1 in {
 def tADJCALLSTACKUP :
   PseudoInst<(outs), (ins i32imm:$amt1, i32imm:$amt2), NoItinerary,

>From 180a2e57c608bd205abd7ad59f4fc16e59742c95 Mon Sep 17 00:00:00 2001
From: AZero13 <gfunni234 at gmail.com>
Date: Mon, 18 Aug 2025 11:46:25 -0400
Subject: [PATCH 4/4] Add comment back

---
 llvm/lib/Target/ARM/ARMInstrInfo.td | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/llvm/lib/Target/ARM/ARMInstrInfo.td b/llvm/lib/Target/ARM/ARMInstrInfo.td
index a54672c460476..231c26abf4143 100644
--- a/llvm/lib/Target/ARM/ARMInstrInfo.td
+++ b/llvm/lib/Target/ARM/ARMInstrInfo.td
@@ -2202,6 +2202,9 @@ def JUMPTABLE_TBH :
 PseudoInst<(outs), (ins cpinst_operand:$instid, cpinst_operand:$cpidx,
                         i32imm:$size), NoItinerary, []>;
 
+// FIXME: Marking these as hasSideEffects is necessary to prevent machine DCE
+// from removing one half of the matched pairs. That breaks PEI, which assumes
+// these will always be in pairs, and asserts if it finds otherwise. Better way?
 let Defs = [SP], Uses = [SP], hasSideEffects = 1 in {
 def ADJCALLSTACKUP :
 PseudoInst<(outs), (ins i32imm:$amt1, i32imm:$amt2, pred:$p), NoItinerary,