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

[SelectionDAG][RISCV][PowerPC][X86] Use TargetConstant for immediates for ISD::PREFETCH. #66601

Merged
merged 2 commits into from
Sep 18, 2023

Conversation

topperc
Copy link
Collaborator

@topperc topperc commented Sep 17, 2023

The intrinsic uses ImmArg so TargetConstant would be consistent
with how other intrinsics are handled.

This hides the constants from type legalization so we can remove
the promotion support.

isel patterns are updated accordingly.

… for ISD::PREFETCH.

The intrinsic uses ImmArg so TargetConstant would be consistent
with how other intrinsics are handled.

This hides the constants from type legalization so we can remove
the promotion support.

isel patterns are updated accordingly.
@llvmbot
Copy link
Member

llvmbot commented Sep 17, 2023

@llvm/pr-subscribers-backend-risc-v
@llvm/pr-subscribers-backend-x86

@llvm/pr-subscribers-llvm-selectiondag

Changes

The intrinsic uses ImmArg so TargetConstant would be consistent
with how other intrinsics are handled.

This hides the constants from type legalization so we can remove
the promotion support.

isel patterns are updated accordingly.


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

7 Files Affected:

  • (modified) llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp (-14)
  • (modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp (+6-3)
  • (modified) llvm/lib/Target/PowerPC/PPCInstrInfo.td (+3-3)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfoZicbo.td (+3-3)
  • (modified) llvm/lib/Target/X86/X86Instr3DNow.td (+3-3)
  • (modified) llvm/lib/Target/X86/X86InstrSSE.td (+4-4)
  • (modified) llvm/test/CodeGen/PowerPC/ppc64-icbt-pwr7.ll (+3-3)
diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp b/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
index 888cb187a5b3fb5..fc9e3ff3734989d 100644
--- a/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
@@ -1818,8 +1818,6 @@ bool DAGTypeLegalizer::PromoteIntegerOperand(SDNode *N, unsigned OpNo) {
   case ISD::FRAMEADDR:
   case ISD::RETURNADDR: Res = PromoteIntOp_FRAMERETURNADDR(N); break;
 
-  case ISD::PREFETCH: Res = PromoteIntOp_PREFETCH(N, OpNo); break;
-
   case ISD::SMULFIX:
   case ISD::SMULFIXSAT:
   case ISD::UMULFIX:
@@ -2333,18 +2331,6 @@ SDValue DAGTypeLegalizer::PromoteIntOp_FRAMERETURNADDR(SDNode *N) {
   return SDValue(DAG.UpdateNodeOperands(N, Op), 0);
 }
 
-SDValue DAGTypeLegalizer::PromoteIntOp_PREFETCH(SDNode *N, unsigned OpNo) {
-  assert(OpNo > 1 && "Don't know how to promote this operand!");
-  // Promote the rw, locality, and cache type arguments to a supported integer
-  // width.
-  SDValue Op2 = ZExtPromotedInteger(N->getOperand(2));
-  SDValue Op3 = ZExtPromotedInteger(N->getOperand(3));
-  SDValue Op4 = ZExtPromotedInteger(N->getOperand(4));
-  return SDValue(DAG.UpdateNodeOperands(N, N->getOperand(0), N->getOperand(1),
-                                        Op2, Op3, Op4),
-                 0);
-}
-
 SDValue DAGTypeLegalizer::PromoteIntOp_ExpOp(SDNode *N) {
   bool IsStrict = N->isStrictFPOpcode();
   SDValue Chain = IsStrict ? N->getOperand(0) : SDValue();
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index 7a85f19f3df5d3e..f39b62abdd87790 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -7089,9 +7089,12 @@ void SelectionDAGBuilder::visitIntrinsicCall(const CallInst &I,
     auto Flags = rw == 0 ? MachineMemOperand::MOLoad :MachineMemOperand::MOStore;
     Ops[0] = DAG.getRoot();
     Ops[1] = getValue(I.getArgOperand(0));
-    Ops[2] = getValue(I.getArgOperand(1));
-    Ops[3] = getValue(I.getArgOperand(2));
-    Ops[4] = getValue(I.getArgOperand(3));
+    Ops[2] = DAG.getTargetConstant(*cast<ConstantInt>(I.getArgOperand(1)), sdl,
+                                   MVT::i32);
+    Ops[3] = DAG.getTargetConstant(*cast<ConstantInt>(I.getArgOperand(2)), sdl,
+                                   MVT::i32);
+    Ops[4] = DAG.getTargetConstant(*cast<ConstantInt>(I.getArgOperand(3)), sdl,
+                                   MVT::i32);
     SDValue Result = DAG.getMemIntrinsicNode(
         ISD::PREFETCH, sdl, DAG.getVTList(MVT::Other), Ops,
         EVT::getIntegerVT(*Context, 8), MachinePointerInfo(I.getArgOperand(0)),
diff --git a/llvm/lib/Target/PowerPC/PPCInstrInfo.td b/llvm/lib/Target/PowerPC/PPCInstrInfo.td
index e1c513dc52dcee8..0567665fbc7349f 100644
--- a/llvm/lib/Target/PowerPC/PPCInstrInfo.td
+++ b/llvm/lib/Target/PowerPC/PPCInstrInfo.td
@@ -1713,11 +1713,11 @@ def : Pat<(int_ppc_dcbf xoaddr:$dst),
 def : Pat<(int_ppc_icbt xoaddr:$dst),
           (ICBT 0, xoaddr:$dst)>;
 
-def : Pat<(prefetch xoaddr:$dst, (i32 0), imm, (i32 1)),
+def : Pat<(prefetch xoaddr:$dst, (i32 0), timm, (i32 1)),
           (DCBT 0, xoaddr:$dst)>;   // data prefetch for loads
-def : Pat<(prefetch xoaddr:$dst, (i32 1), imm, (i32 1)),
+def : Pat<(prefetch xoaddr:$dst, (i32 1), timm, (i32 1)),
           (DCBTST 0, xoaddr:$dst)>; // data prefetch for stores
-def : Pat<(prefetch xoaddr:$dst, (i32 0), imm, (i32 0)),
+def : Pat<(prefetch xoaddr:$dst, (i32 0), timm, (i32 0)),
           (ICBT 0, xoaddr:$dst)>, Requires<[HasICBT]>; // inst prefetch (for read)
 
 def : Pat<(int_ppc_dcbt_with_hint xoaddr:$dst, i32:$TH),
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoZicbo.td b/llvm/lib/Target/RISCV/RISCVInstrInfoZicbo.td
index 509d1cfcd874444..6c0fb29bac214e3 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoZicbo.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoZicbo.td
@@ -76,10 +76,10 @@ def PREFETCH_W : Prefetch_ri<0b00011, "prefetch.w">, Sched<[]>;
 
 let Predicates = [HasStdExtZicbop] in {
   // FIXME: Match address with offset
-  def : Pat<(prefetch GPR:$rs1, imm, imm, (XLenVT 0)),
+  def : Pat<(prefetch GPR:$rs1, timm, timm, (i32 0)),
             (PREFETCH_I GPR:$rs1, 0)>;
-  def : Pat<(prefetch GPR:$rs1, (XLenVT 0), imm, (XLenVT 1)),
+  def : Pat<(prefetch GPR:$rs1, (i32 0), timm, (i32 1)),
             (PREFETCH_R GPR:$rs1, 0)>;
-  def : Pat<(prefetch GPR:$rs1, (XLenVT 1), imm, (XLenVT 1)),
+  def : Pat<(prefetch GPR:$rs1, (i32 1), timm, (i32 1)),
             (PREFETCH_W GPR:$rs1, 0)>;
 }
diff --git a/llvm/lib/Target/X86/X86Instr3DNow.td b/llvm/lib/Target/X86/X86Instr3DNow.td
index cd1b0636597126a..d5651b67769570b 100644
--- a/llvm/lib/Target/X86/X86Instr3DNow.td
+++ b/llvm/lib/Target/X86/X86Instr3DNow.td
@@ -80,12 +80,12 @@ def FEMMS : I3DNow<0x0E, RawFrm, (outs), (ins), "femms",
                    [(int_x86_mmx_femms)]>, TB;
 
 // PREFETCHWT1 is supported we want to use it for everything but T0.
-def PrefetchWLevel : PatFrag<(ops), (i32 imm), [{
+def PrefetchWLevel : PatFrag<(ops), (i32 timm), [{
   return N->getSExtValue() == 3 || !Subtarget->hasPREFETCHWT1();
 }]>;
 
 // Use PREFETCHWT1 for NTA, T2, T1.
-def PrefetchWT1Level : ImmLeaf<i32, [{
+def PrefetchWT1Level : TImmLeaf<i32, [{
   return Imm < 3;
 }]>;
 
@@ -93,7 +93,7 @@ let SchedRW = [WriteLoad] in {
 let Predicates = [Has3DNow, NoSSEPrefetch] in
 def PREFETCH : I3DNow<0x0D, MRM0m, (outs), (ins i8mem:$addr),
                       "prefetch\t$addr",
-                      [(prefetch addr:$addr, imm, imm, (i32 1))]>, TB;
+                      [(prefetch addr:$addr, timm, timm, (i32 1))]>, TB;
 
 def PREFETCHW : I<0x0D, MRM1m, (outs), (ins i8mem:$addr), "prefetchw\t$addr",
                   [(prefetch addr:$addr, (i32 1), (i32 PrefetchWLevel), (i32 1))]>,
diff --git a/llvm/lib/Target/X86/X86InstrSSE.td b/llvm/lib/Target/X86/X86InstrSSE.td
index a6fcc804e1d0635..2ba1436946a286a 100644
--- a/llvm/lib/Target/X86/X86InstrSSE.td
+++ b/llvm/lib/Target/X86/X86InstrSSE.td
@@ -3212,13 +3212,13 @@ let Predicates = [UseSSE2] in {
 // Prefetch intrinsic.
 let Predicates = [HasSSEPrefetch], SchedRW = [WriteLoad] in {
 def PREFETCHT0   : I<0x18, MRM1m, (outs), (ins i8mem:$src),
-    "prefetcht0\t$src", [(prefetch addr:$src, imm, (i32 3), (i32 1))]>, TB;
+    "prefetcht0\t$src", [(prefetch addr:$src, timm, (i32 3), (i32 1))]>, TB;
 def PREFETCHT1   : I<0x18, MRM2m, (outs), (ins i8mem:$src),
-    "prefetcht1\t$src", [(prefetch addr:$src, imm, (i32 2), (i32 1))]>, TB;
+    "prefetcht1\t$src", [(prefetch addr:$src, timm, (i32 2), (i32 1))]>, TB;
 def PREFETCHT2   : I<0x18, MRM3m, (outs), (ins i8mem:$src),
-    "prefetcht2\t$src", [(prefetch addr:$src, imm, (i32 1), (i32 1))]>, TB;
+    "prefetcht2\t$src", [(prefetch addr:$src, timm, (i32 1), (i32 1))]>, TB;
 def PREFETCHNTA  : I<0x18, MRM0m, (outs), (ins i8mem:$src),
-    "prefetchnta\t$src", [(prefetch addr:$src, imm, (i32 0), (i32 1))]>, TB;
+    "prefetchnta\t$src", [(prefetch addr:$src, timm, (i32 0), (i32 1))]>, TB;
 }
 
 // FIXME: How should flush instruction be modeled?
diff --git a/llvm/test/CodeGen/PowerPC/ppc64-icbt-pwr7.ll b/llvm/test/CodeGen/PowerPC/ppc64-icbt-pwr7.ll
index 760f08c7166fdcc..c9de1d8fc433428 100644
--- a/llvm/test/CodeGen/PowerPC/ppc64-icbt-pwr7.ll
+++ b/llvm/test/CodeGen/PowerPC/ppc64-icbt-pwr7.ll
@@ -11,9 +11,9 @@ entry:
 
 ; FIXME: Crashing is not really the correct behavior here, we really should just emit nothing
 ; CHECK: Cannot select: {{0x[0-9,a-f]+|t[0-9]+}}: ch = Prefetch
-; CHECK: {{0x[0-9,a-f]+|t[0-9]+}}: i32 = Constant<0>
-; CHECK-NEXT: {{0x[0-9,a-f]+|t[0-9]+}}: i32 = Constant<3>
-; CHECK-NEXT: {{0x[0-9,a-f]+|t[0-9]+}}: i32 = Constant<0>
+; CHECK: {{0x[0-9,a-f]+|t[0-9]+}}: i32 = TargetConstant<0>
+; CHECK-NEXT: {{0x[0-9,a-f]+|t[0-9]+}}: i32 = TargetConstant<3>
+; CHECK-NEXT: {{0x[0-9,a-f]+|t[0-9]+}}: i32 = TargetConstant<0>
 
 }
 

…ediates for ISD::PREFETCH.

Remove unused declaration
Copy link
Contributor

@phoebewang phoebewang left a comment

Choose a reason for hiding this comment

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

X86 changes look good to me.

Copy link
Collaborator

@RKSimon RKSimon left a comment

Choose a reason for hiding this comment

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

LGTM - cheers

@topperc topperc merged commit f71a9e8 into llvm:main Sep 18, 2023
@topperc topperc deleted the pr/prefetch-timm branch September 18, 2023 15:58
ZijunZhaoCCK pushed a commit to ZijunZhaoCCK/llvm-project that referenced this pull request Sep 19, 2023
… for ISD::PREFETCH. (llvm#66601)

The intrinsic uses ImmArg so TargetConstant would be consistent
with how other intrinsics are handled.

This hides the constants from type legalization so we can remove
the promotion support.

isel patterns are updated accordingly.
zahiraam pushed a commit to tahonermann/llvm-project that referenced this pull request Oct 24, 2023
… for ISD::PREFETCH. (llvm#66601)

The intrinsic uses ImmArg so TargetConstant would be consistent
with how other intrinsics are handled.

This hides the constants from type legalization so we can remove
the promotion support.

isel patterns are updated accordingly.
zahiraam pushed a commit to tahonermann/llvm-project that referenced this pull request Oct 24, 2023
… for ISD::PREFETCH. (llvm#66601)

The intrinsic uses ImmArg so TargetConstant would be consistent
with how other intrinsics are handled.

This hides the constants from type legalization so we can remove
the promotion support.

isel patterns are updated accordingly.
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