Skip to content

SelectionDAG: Add missing AddNodeIDCustom case for MDNodeSDNode. #136805

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

Conversation

pcc
Copy link
Contributor

@pcc pcc commented Apr 23, 2025

Without this we ended up never deduplicating MDNodeSDNodes.

Created using spr 1.3.6-beta.1
@llvmbot llvmbot added the llvm:SelectionDAG SelectionDAGISel as well label Apr 23, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 23, 2025

@llvm/pr-subscribers-llvm-selectiondag

Author: Peter Collingbourne (pcc)

Changes

Without this we ended up never deduplicating MDNodeSDNodes.


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

1 Files Affected:

  • (modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp (+3)
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
index a41efd58ce4e4..0a2687a16a80c 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -967,6 +967,9 @@ static void AddNodeIDCustom(FoldingSetNodeID &ID, const SDNode *N) {
   case ISD::INTRINSIC_W_CHAIN:
     // Handled by MemIntrinsicSDNode check after the switch.
     break;
+  case ISD::MDNODE_SDNODE:
+    ID.AddPointer(cast<MDNodeSDNode>(N)->getMD());
+    break;
   } // end switch (N->getOpcode())
 
   // MemIntrinsic nodes could also have subclass data, address spaces, and flags

@pcc pcc requested a review from arsenm April 23, 2025 04:00
@arsenm
Copy link
Contributor

arsenm commented Apr 23, 2025

Testcase?

@pcc
Copy link
Contributor Author

pcc commented Apr 23, 2025

Testcase?

The only symptom of this bug that I'm aware of is that the backend is slower (because the nodes are never deduplicated) and I'm not sure that we can write a test case for that.

@pcc pcc merged commit dbb8434 into main Apr 23, 2025
13 checks passed
@pcc pcc deleted the users/pcc/spr/selectiondag-add-missing-addnodeidcustom-case-for-mdnodesdnode branch April 23, 2025 18:00
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request May 6, 2025
…DNode.

Without this we ended up never deduplicating MDNodeSDNodes.

Reviewers: arsenm

Reviewed By: arsenm

Pull Request: llvm/llvm-project#136805
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
Without this we ended up never deduplicating MDNodeSDNodes.

Reviewers: arsenm

Reviewed By: arsenm

Pull Request: llvm#136805
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
Without this we ended up never deduplicating MDNodeSDNodes.

Reviewers: arsenm

Reviewed By: arsenm

Pull Request: llvm#136805
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
Without this we ended up never deduplicating MDNodeSDNodes.

Reviewers: arsenm

Reviewed By: arsenm

Pull Request: llvm#136805
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm:SelectionDAG SelectionDAGISel as well
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants