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

add ucmp and scmp support to SelectionDAG #85822

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

miguelraz
Copy link
Contributor

@miguelraz miguelraz commented Mar 19, 2024

This PR is the next step after #83227 in a possible GSoC project to add 3 way comparison intrinsics to LLVM IR.

Steps as outlined in the documentation

  1. Add an enum value for the SelectionDAG node in .../ISDOpcodes.h
  2. CodeGen/SelectionDAG/SelectionDAG.cpp - add code to print the node to getOperationName
  3. .../SelectionDAG/LegalizeDAG.cpp - add legalize, promote, expand directives as necessary
  4. ibid - size promotion of only applicable to same sizes
  5. ibid - add a case for node in ExpandOp to teach legalizer for 64bits in 32bit targets
  6. SDAG/DAGCombiner.cpp - add a visit function (see visitFABS and visitSRL as starting places
  7. llvm/Target/PowerPC/PPCISelLowering.cpp - add native support if not default
  8. Target/TargetSelectionDAG.td - add a def to that node with appropriate type constrains (see add/bswap/fadd)
  9. lib/Target/PowerPC/PPCInstrInfo.td - add a pattern for new node that uses one or more target nodes (see rotl in PPCInstrInfo.td)
  10. add complex patterns?
  11. test/CodeGen/* add unit tests as you go

cc @nikic and @dc03-work

Copy link

github-actions bot commented Mar 19, 2024

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff b9d83eff254668385fd3d9d5ddb5af762f378d7f 6ca1249f20c7efb8431a4542b35e4d670e2a7de0 -- llvm/include/llvm/CodeGen/ISDOpcodes.h llvm/include/llvm/CodeGen/TargetLowering.h llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp llvm/unittests/Analysis/ValueTrackingTest.cpp
View the diff from clang-format here.
diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
index b4f1fa1f90..0afc9facf5 100644
--- a/llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
@@ -3584,7 +3584,7 @@ bool SelectionDAGLegalize::ExpandNode(SDNode *Node) {
   }
   case ISD::UCMP:
   case ISD::SCMP:
-  // FIX: add logic here
+    // FIX: add logic here
     break;
   case ISD::FMINNUM:
   case ISD::FMAXNUM: {
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
index beb152b419..2824bcce57 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -6707,13 +6707,16 @@ SDValue SelectionDAG::getNode(unsigned Opcode, const SDLoc &DL, EVT VT,
   case ISD::UCMP:
   case ISD::SCMP:
     // FIX: This cast is clearly wrong
-    assert(cast<SignedInt>(N1.getValueType) && "This operator should have signed types");
+    assert(cast<SignedInt>(N1.getValueType) &&
+           "This operator should have signed types");
     assert(VT.isInteger() && "This operator does not apply to FP types!");
     assert(N1.getValueType() == N2.getValueType() && N1.getValueType() == VT &&
-	       "Binary operator types must match");
-    // FIX: This logic should probably go in a separate function to deduplicate it from ucmp. Suggestions?
+           "Binary operator types must match");
+    // FIX: This logic should probably go in a separate function to deduplicate
+    // it from ucmp. Suggestions?
     if (N1C > N2C) {
-      // FIX: All of these casts are horrible, I couldn't find the proper way to fold the constants in
+      // FIX: All of these casts are horrible, I couldn't find the proper way to
+      // fold the constants in
       return cast<ConstantInt>(1);
     }
     if (N1C == N2C) {
@@ -6722,7 +6725,7 @@ SDValue SelectionDAG::getNode(unsigned Opcode, const SDLoc &DL, EVT VT,
     if (N1C < N2C) {
       return cast<ConstantInt>(-1);
     }
-      break;
+    break;
   case ISD::MUL:
     assert(VT.isInteger() && "This operator does not apply to FP types!");
     assert(N1.getValueType() == N2.getValueType() &&

Copy link

✅ With the latest revision this PR passed the Python code formatter.

@miguelraz
Copy link
Contributor Author

@nikic I pushed this as far as I could today to try and solve the add code to print the node to 'getOperationName' and fold the constants in.

It's probably atrocious code but I hit a wall of

  • not knowing how to return a constant SDNode of value 1 or 0
  • not knowing how to check if N1 was signed or not

And I still am struggling with the SVNode APIs.

@dtcxzyw
Copy link
Member

dtcxzyw commented Apr 13, 2024

Ping @nikic

@miguelraz
Copy link
Contributor Author

Seeing as how @Poseydon42 is going to be taking over the GSoC, I'd be happy to have them push this PR forward or based on top of it. Feel free to ping me on the LLVM Discord/Discourse if I can help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants