Skip to content

Conversation

@mohammadamin382
Copy link

EnforceNodeIdInvariant currently processes nodes with multiple predecessors
multiple times in reconvergent DAGs due to lacking a visited set.

This inefficient O(V*E) complexity is optimized to O(V+E) by introducing
a SmallPtrSet to track visited nodes. This ensures each node is processed
only once, improving compile time for highly connected graphs.

@github-actions
Copy link

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@llvmbot
Copy link
Member

llvmbot commented Oct 23, 2025

@llvm/pr-subscribers-llvm-selectiondag

@llvm/pr-subscribers-backend-aarch64

Author: Mohammad (mohammadamin382)

Changes

EnforceNodeIdInvariant currently processes nodes with multiple predecessors
multiple times in reconvergent DAGs due to lacking a visited set.

This inefficient O(V*E) complexity is optimized to O(V+E) by introducing
a SmallPtrSet to track visited nodes. This ensures each node is processed
only once, improving compile time for highly connected graphs.


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

3 Files Affected:

  • (modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp (+3-1)
  • (added) llvm/test/CodeGen/AArch64/test-enforce-nodeid-invariant.ll (+119)
  • (added) llvm/test/CodeGen/X86/test-enforce-nodeid-invariant.ll (+119)
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
index 6c11c5b815b6b..8fb4586333840 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
@@ -1236,13 +1236,15 @@ class ISelUpdater : public SelectionDAG::DAGUpdateListener {
 // functions.
 void SelectionDAGISel::EnforceNodeIdInvariant(SDNode *Node) {
   SmallVector<SDNode *, 4> Nodes;
+  SmallPtrSet<SDNode *, 16> Visited;
   Nodes.push_back(Node);
+  Visited.insert(Node);
 
   while (!Nodes.empty()) {
     SDNode *N = Nodes.pop_back_val();
     for (auto *U : N->users()) {
       auto UId = U->getNodeId();
-      if (UId > 0) {
+      if (UId > 0 && Visited.insert(U).second) {
         InvalidateNodeId(U);
         Nodes.push_back(U);
       }
diff --git a/llvm/test/CodeGen/AArch64/test-enforce-nodeid-invariant.ll b/llvm/test/CodeGen/AArch64/test-enforce-nodeid-invariant.ll
new file mode 100644
index 0000000000000..9b70dd9dcd0a7
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/test-enforce-nodeid-invariant.ll
@@ -0,0 +1,119 @@
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -verify-machineinstrs | FileCheck %s
+; RUN: llc < %s -mtriple=aarch64-unknown-linux-gnu -verify-machineinstrs | FileCheck %s
+
+; Test that EnforceNodeIdInvariant correctly handles nodes
+; users and prevents redundant invalidation visits. This test creates a
+; DAG structure with high fan-in where multiple nodes share common users.
+; should visit each node exactly once.
+
+define i32 @test_diamond_dag(i32 %a, i32 %b) {
+; CHECK-LABEL: test_diamond_dag:
+entry:
+  %add1 = add i32 %a, %b
+  %add2 = add i32 %a, %b
+  %mul1 = mul i32 %add1, %add2
+  %mul2 = mul i32 %add1, %add2
+  %result = add i32 %mul1, %mul2
+  ret i32 %result
+}
+
+define i64 @test_deep_dag(i64 %x) {
+; CHECK-LABEL: test_deep_dag:
+entry:
+  %a = add i64 %x, 1
+  %b = add i64 %x, 2
+  %c = add i64 %a, %b
+  %d = add i64 %a, %b
+  %e = mul i64 %c, %d
+  %f = mul i64 %c, %d
+  %g = add i64 %e, %f
+  ret i64 %g
+}
+
+define i32 @test_wide_dag(i32 %v) {
+; CHECK-LABEL: test_wide_dag:
+entry:
+  %a = add i32 %v, 1
+  %b = add i32 %v, 2
+  %c = add i32 %v, 3
+  %d = add i32 %v, 4
+  %ab = add i32 %a, %b
+  %cd = add i32 %c, %d
+  %ac = add i32 %a, %c
+  %bd = add i32 %b, %d
+  %t1 = mul i32 %ab, %cd
+  %t2 = mul i32 %ac, %bd
+  %result = add i32 %t1, %t2
+  ret i32 %result
+}
+
+define <4 x i32> @test_vector_dag(<4 x i32> %vec1, <4 x i32> %vec2) {
+; CHECK-LABEL: test_vector_dag:
+entry:
+  %add1 = add <4 x i32> %vec1, %vec2
+  %add2 = add <4 x i32> %vec1, %vec2
+  %mul1 = mul <4 x i32> %add1, %add2
+  %mul2 = mul <4 x i32> %add1, %add2
+  %result = add <4 x i32> %mul1, %mul2
+  ret <4 x i32> %result
+}
+
+define i32 @test_chain_with_shared_users(i32 %x, i32 %y, i32 %z) {
+; CHECK-LABEL: test_chain_with_shared_users:
+entry:
+  %a = add i32 %x, %y
+  %b = add i32 %y, %z
+  %c = mul i32 %a, %b
+  %d = mul i32 %a, %b
+  %e = add i32 %c, %d
+  %f = sub i32 %c, %d
+  %g = mul i32 %e, %f
+  ret i32 %g
+}
+
+define i64 @test_complex_sharing(i64 %p1, i64 %p2, i64 %p3) {
+; CHECK-LABEL: test_complex_sharing:
+entry:
+  %n1 = add i64 %p1, %p2
+  %n2 = add i64 %p2, %p3
+  %n3 = add i64 %p1, %p3
+  %u1 = mul i64 %n1, %n2
+  %u2 = mul i64 %n1, %n3
+  %u3 = mul i64 %n2, %n3
+  %s1 = add i64 %u1, %u2
+  %s2 = add i64 %u2, %u3
+  %s3 = add i64 %u1, %u3
+  %f1 = mul i64 %s1, %s2
+  %f2 = mul i64 %s2, %s3
+  %result = add i64 %f1, %f2
+  ret i64 %result
+}
+
+define i32 @test_multiple_paths_to_same_user(i32 %input) {
+; CHECK-LABEL: test_multiple_paths_to_same_user:
+entry:
+  %a = add i32 %input, 1
+  %b = add i32 %input, 2
+  %c = add i32 %input, 3
+  %ab = mul i32 %a, %b
+  %bc = mul i32 %b, %c
+  %ac = mul i32 %a, %c
+  %shared = add i32 %ab, %bc
+  %shared2 = add i32 %ac, %shared
+  ret i32 %shared2
+}
+
+define i64 @test_reconvergent_paths(i64 %val) {
+; CHECK-LABEL: test_reconvergent_paths:
+entry:
+  %l1a = add i64 %val, 10
+  %l1b = add i64 %val, 20
+  %l2a = mul i64 %l1a, 2
+  %l2b = mul i64 %l1b, 3
+  %l2c = mul i64 %l1a, 4
+  %l2d = mul i64 %l1b, 5
+  %l3a = add i64 %l2a, %l2b
+  %l3b = add i64 %l2c, %l2d
+  %merge = mul i64 %l3a, %l3b
+  ret i64 %merge
+}
diff --git a/llvm/test/CodeGen/X86/test-enforce-nodeid-invariant.ll b/llvm/test/CodeGen/X86/test-enforce-nodeid-invariant.ll
new file mode 100644
index 0000000000000..9b70dd9dcd0a7
--- /dev/null
+++ b/llvm/test/CodeGen/X86/test-enforce-nodeid-invariant.ll
@@ -0,0 +1,119 @@
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -verify-machineinstrs | FileCheck %s
+; RUN: llc < %s -mtriple=aarch64-unknown-linux-gnu -verify-machineinstrs | FileCheck %s
+
+; Test that EnforceNodeIdInvariant correctly handles nodes
+; users and prevents redundant invalidation visits. This test creates a
+; DAG structure with high fan-in where multiple nodes share common users.
+; should visit each node exactly once.
+
+define i32 @test_diamond_dag(i32 %a, i32 %b) {
+; CHECK-LABEL: test_diamond_dag:
+entry:
+  %add1 = add i32 %a, %b
+  %add2 = add i32 %a, %b
+  %mul1 = mul i32 %add1, %add2
+  %mul2 = mul i32 %add1, %add2
+  %result = add i32 %mul1, %mul2
+  ret i32 %result
+}
+
+define i64 @test_deep_dag(i64 %x) {
+; CHECK-LABEL: test_deep_dag:
+entry:
+  %a = add i64 %x, 1
+  %b = add i64 %x, 2
+  %c = add i64 %a, %b
+  %d = add i64 %a, %b
+  %e = mul i64 %c, %d
+  %f = mul i64 %c, %d
+  %g = add i64 %e, %f
+  ret i64 %g
+}
+
+define i32 @test_wide_dag(i32 %v) {
+; CHECK-LABEL: test_wide_dag:
+entry:
+  %a = add i32 %v, 1
+  %b = add i32 %v, 2
+  %c = add i32 %v, 3
+  %d = add i32 %v, 4
+  %ab = add i32 %a, %b
+  %cd = add i32 %c, %d
+  %ac = add i32 %a, %c
+  %bd = add i32 %b, %d
+  %t1 = mul i32 %ab, %cd
+  %t2 = mul i32 %ac, %bd
+  %result = add i32 %t1, %t2
+  ret i32 %result
+}
+
+define <4 x i32> @test_vector_dag(<4 x i32> %vec1, <4 x i32> %vec2) {
+; CHECK-LABEL: test_vector_dag:
+entry:
+  %add1 = add <4 x i32> %vec1, %vec2
+  %add2 = add <4 x i32> %vec1, %vec2
+  %mul1 = mul <4 x i32> %add1, %add2
+  %mul2 = mul <4 x i32> %add1, %add2
+  %result = add <4 x i32> %mul1, %mul2
+  ret <4 x i32> %result
+}
+
+define i32 @test_chain_with_shared_users(i32 %x, i32 %y, i32 %z) {
+; CHECK-LABEL: test_chain_with_shared_users:
+entry:
+  %a = add i32 %x, %y
+  %b = add i32 %y, %z
+  %c = mul i32 %a, %b
+  %d = mul i32 %a, %b
+  %e = add i32 %c, %d
+  %f = sub i32 %c, %d
+  %g = mul i32 %e, %f
+  ret i32 %g
+}
+
+define i64 @test_complex_sharing(i64 %p1, i64 %p2, i64 %p3) {
+; CHECK-LABEL: test_complex_sharing:
+entry:
+  %n1 = add i64 %p1, %p2
+  %n2 = add i64 %p2, %p3
+  %n3 = add i64 %p1, %p3
+  %u1 = mul i64 %n1, %n2
+  %u2 = mul i64 %n1, %n3
+  %u3 = mul i64 %n2, %n3
+  %s1 = add i64 %u1, %u2
+  %s2 = add i64 %u2, %u3
+  %s3 = add i64 %u1, %u3
+  %f1 = mul i64 %s1, %s2
+  %f2 = mul i64 %s2, %s3
+  %result = add i64 %f1, %f2
+  ret i64 %result
+}
+
+define i32 @test_multiple_paths_to_same_user(i32 %input) {
+; CHECK-LABEL: test_multiple_paths_to_same_user:
+entry:
+  %a = add i32 %input, 1
+  %b = add i32 %input, 2
+  %c = add i32 %input, 3
+  %ab = mul i32 %a, %b
+  %bc = mul i32 %b, %c
+  %ac = mul i32 %a, %c
+  %shared = add i32 %ab, %bc
+  %shared2 = add i32 %ac, %shared
+  ret i32 %shared2
+}
+
+define i64 @test_reconvergent_paths(i64 %val) {
+; CHECK-LABEL: test_reconvergent_paths:
+entry:
+  %l1a = add i64 %val, 10
+  %l1b = add i64 %val, 20
+  %l2a = mul i64 %l1a, 2
+  %l2b = mul i64 %l1b, 3
+  %l2c = mul i64 %l1a, 4
+  %l2d = mul i64 %l1b, 5
+  %l3a = add i64 %l2a, %l2b
+  %l3b = add i64 %l2c, %l2d
+  %merge = mul i64 %l3a, %l3b
+  ret i64 %merge
+}

@llvmbot
Copy link
Member

llvmbot commented Oct 23, 2025

@llvm/pr-subscribers-backend-x86

Author: Mohammad (mohammadamin382)

Changes

EnforceNodeIdInvariant currently processes nodes with multiple predecessors
multiple times in reconvergent DAGs due to lacking a visited set.

This inefficient O(V*E) complexity is optimized to O(V+E) by introducing
a SmallPtrSet to track visited nodes. This ensures each node is processed
only once, improving compile time for highly connected graphs.


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

3 Files Affected:

  • (modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp (+3-1)
  • (added) llvm/test/CodeGen/AArch64/test-enforce-nodeid-invariant.ll (+119)
  • (added) llvm/test/CodeGen/X86/test-enforce-nodeid-invariant.ll (+119)
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
index 6c11c5b815b6b..8fb4586333840 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
@@ -1236,13 +1236,15 @@ class ISelUpdater : public SelectionDAG::DAGUpdateListener {
 // functions.
 void SelectionDAGISel::EnforceNodeIdInvariant(SDNode *Node) {
   SmallVector<SDNode *, 4> Nodes;
+  SmallPtrSet<SDNode *, 16> Visited;
   Nodes.push_back(Node);
+  Visited.insert(Node);
 
   while (!Nodes.empty()) {
     SDNode *N = Nodes.pop_back_val();
     for (auto *U : N->users()) {
       auto UId = U->getNodeId();
-      if (UId > 0) {
+      if (UId > 0 && Visited.insert(U).second) {
         InvalidateNodeId(U);
         Nodes.push_back(U);
       }
diff --git a/llvm/test/CodeGen/AArch64/test-enforce-nodeid-invariant.ll b/llvm/test/CodeGen/AArch64/test-enforce-nodeid-invariant.ll
new file mode 100644
index 0000000000000..9b70dd9dcd0a7
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/test-enforce-nodeid-invariant.ll
@@ -0,0 +1,119 @@
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -verify-machineinstrs | FileCheck %s
+; RUN: llc < %s -mtriple=aarch64-unknown-linux-gnu -verify-machineinstrs | FileCheck %s
+
+; Test that EnforceNodeIdInvariant correctly handles nodes
+; users and prevents redundant invalidation visits. This test creates a
+; DAG structure with high fan-in where multiple nodes share common users.
+; should visit each node exactly once.
+
+define i32 @test_diamond_dag(i32 %a, i32 %b) {
+; CHECK-LABEL: test_diamond_dag:
+entry:
+  %add1 = add i32 %a, %b
+  %add2 = add i32 %a, %b
+  %mul1 = mul i32 %add1, %add2
+  %mul2 = mul i32 %add1, %add2
+  %result = add i32 %mul1, %mul2
+  ret i32 %result
+}
+
+define i64 @test_deep_dag(i64 %x) {
+; CHECK-LABEL: test_deep_dag:
+entry:
+  %a = add i64 %x, 1
+  %b = add i64 %x, 2
+  %c = add i64 %a, %b
+  %d = add i64 %a, %b
+  %e = mul i64 %c, %d
+  %f = mul i64 %c, %d
+  %g = add i64 %e, %f
+  ret i64 %g
+}
+
+define i32 @test_wide_dag(i32 %v) {
+; CHECK-LABEL: test_wide_dag:
+entry:
+  %a = add i32 %v, 1
+  %b = add i32 %v, 2
+  %c = add i32 %v, 3
+  %d = add i32 %v, 4
+  %ab = add i32 %a, %b
+  %cd = add i32 %c, %d
+  %ac = add i32 %a, %c
+  %bd = add i32 %b, %d
+  %t1 = mul i32 %ab, %cd
+  %t2 = mul i32 %ac, %bd
+  %result = add i32 %t1, %t2
+  ret i32 %result
+}
+
+define <4 x i32> @test_vector_dag(<4 x i32> %vec1, <4 x i32> %vec2) {
+; CHECK-LABEL: test_vector_dag:
+entry:
+  %add1 = add <4 x i32> %vec1, %vec2
+  %add2 = add <4 x i32> %vec1, %vec2
+  %mul1 = mul <4 x i32> %add1, %add2
+  %mul2 = mul <4 x i32> %add1, %add2
+  %result = add <4 x i32> %mul1, %mul2
+  ret <4 x i32> %result
+}
+
+define i32 @test_chain_with_shared_users(i32 %x, i32 %y, i32 %z) {
+; CHECK-LABEL: test_chain_with_shared_users:
+entry:
+  %a = add i32 %x, %y
+  %b = add i32 %y, %z
+  %c = mul i32 %a, %b
+  %d = mul i32 %a, %b
+  %e = add i32 %c, %d
+  %f = sub i32 %c, %d
+  %g = mul i32 %e, %f
+  ret i32 %g
+}
+
+define i64 @test_complex_sharing(i64 %p1, i64 %p2, i64 %p3) {
+; CHECK-LABEL: test_complex_sharing:
+entry:
+  %n1 = add i64 %p1, %p2
+  %n2 = add i64 %p2, %p3
+  %n3 = add i64 %p1, %p3
+  %u1 = mul i64 %n1, %n2
+  %u2 = mul i64 %n1, %n3
+  %u3 = mul i64 %n2, %n3
+  %s1 = add i64 %u1, %u2
+  %s2 = add i64 %u2, %u3
+  %s3 = add i64 %u1, %u3
+  %f1 = mul i64 %s1, %s2
+  %f2 = mul i64 %s2, %s3
+  %result = add i64 %f1, %f2
+  ret i64 %result
+}
+
+define i32 @test_multiple_paths_to_same_user(i32 %input) {
+; CHECK-LABEL: test_multiple_paths_to_same_user:
+entry:
+  %a = add i32 %input, 1
+  %b = add i32 %input, 2
+  %c = add i32 %input, 3
+  %ab = mul i32 %a, %b
+  %bc = mul i32 %b, %c
+  %ac = mul i32 %a, %c
+  %shared = add i32 %ab, %bc
+  %shared2 = add i32 %ac, %shared
+  ret i32 %shared2
+}
+
+define i64 @test_reconvergent_paths(i64 %val) {
+; CHECK-LABEL: test_reconvergent_paths:
+entry:
+  %l1a = add i64 %val, 10
+  %l1b = add i64 %val, 20
+  %l2a = mul i64 %l1a, 2
+  %l2b = mul i64 %l1b, 3
+  %l2c = mul i64 %l1a, 4
+  %l2d = mul i64 %l1b, 5
+  %l3a = add i64 %l2a, %l2b
+  %l3b = add i64 %l2c, %l2d
+  %merge = mul i64 %l3a, %l3b
+  ret i64 %merge
+}
diff --git a/llvm/test/CodeGen/X86/test-enforce-nodeid-invariant.ll b/llvm/test/CodeGen/X86/test-enforce-nodeid-invariant.ll
new file mode 100644
index 0000000000000..9b70dd9dcd0a7
--- /dev/null
+++ b/llvm/test/CodeGen/X86/test-enforce-nodeid-invariant.ll
@@ -0,0 +1,119 @@
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -verify-machineinstrs | FileCheck %s
+; RUN: llc < %s -mtriple=aarch64-unknown-linux-gnu -verify-machineinstrs | FileCheck %s
+
+; Test that EnforceNodeIdInvariant correctly handles nodes
+; users and prevents redundant invalidation visits. This test creates a
+; DAG structure with high fan-in where multiple nodes share common users.
+; should visit each node exactly once.
+
+define i32 @test_diamond_dag(i32 %a, i32 %b) {
+; CHECK-LABEL: test_diamond_dag:
+entry:
+  %add1 = add i32 %a, %b
+  %add2 = add i32 %a, %b
+  %mul1 = mul i32 %add1, %add2
+  %mul2 = mul i32 %add1, %add2
+  %result = add i32 %mul1, %mul2
+  ret i32 %result
+}
+
+define i64 @test_deep_dag(i64 %x) {
+; CHECK-LABEL: test_deep_dag:
+entry:
+  %a = add i64 %x, 1
+  %b = add i64 %x, 2
+  %c = add i64 %a, %b
+  %d = add i64 %a, %b
+  %e = mul i64 %c, %d
+  %f = mul i64 %c, %d
+  %g = add i64 %e, %f
+  ret i64 %g
+}
+
+define i32 @test_wide_dag(i32 %v) {
+; CHECK-LABEL: test_wide_dag:
+entry:
+  %a = add i32 %v, 1
+  %b = add i32 %v, 2
+  %c = add i32 %v, 3
+  %d = add i32 %v, 4
+  %ab = add i32 %a, %b
+  %cd = add i32 %c, %d
+  %ac = add i32 %a, %c
+  %bd = add i32 %b, %d
+  %t1 = mul i32 %ab, %cd
+  %t2 = mul i32 %ac, %bd
+  %result = add i32 %t1, %t2
+  ret i32 %result
+}
+
+define <4 x i32> @test_vector_dag(<4 x i32> %vec1, <4 x i32> %vec2) {
+; CHECK-LABEL: test_vector_dag:
+entry:
+  %add1 = add <4 x i32> %vec1, %vec2
+  %add2 = add <4 x i32> %vec1, %vec2
+  %mul1 = mul <4 x i32> %add1, %add2
+  %mul2 = mul <4 x i32> %add1, %add2
+  %result = add <4 x i32> %mul1, %mul2
+  ret <4 x i32> %result
+}
+
+define i32 @test_chain_with_shared_users(i32 %x, i32 %y, i32 %z) {
+; CHECK-LABEL: test_chain_with_shared_users:
+entry:
+  %a = add i32 %x, %y
+  %b = add i32 %y, %z
+  %c = mul i32 %a, %b
+  %d = mul i32 %a, %b
+  %e = add i32 %c, %d
+  %f = sub i32 %c, %d
+  %g = mul i32 %e, %f
+  ret i32 %g
+}
+
+define i64 @test_complex_sharing(i64 %p1, i64 %p2, i64 %p3) {
+; CHECK-LABEL: test_complex_sharing:
+entry:
+  %n1 = add i64 %p1, %p2
+  %n2 = add i64 %p2, %p3
+  %n3 = add i64 %p1, %p3
+  %u1 = mul i64 %n1, %n2
+  %u2 = mul i64 %n1, %n3
+  %u3 = mul i64 %n2, %n3
+  %s1 = add i64 %u1, %u2
+  %s2 = add i64 %u2, %u3
+  %s3 = add i64 %u1, %u3
+  %f1 = mul i64 %s1, %s2
+  %f2 = mul i64 %s2, %s3
+  %result = add i64 %f1, %f2
+  ret i64 %result
+}
+
+define i32 @test_multiple_paths_to_same_user(i32 %input) {
+; CHECK-LABEL: test_multiple_paths_to_same_user:
+entry:
+  %a = add i32 %input, 1
+  %b = add i32 %input, 2
+  %c = add i32 %input, 3
+  %ab = mul i32 %a, %b
+  %bc = mul i32 %b, %c
+  %ac = mul i32 %a, %c
+  %shared = add i32 %ab, %bc
+  %shared2 = add i32 %ac, %shared
+  ret i32 %shared2
+}
+
+define i64 @test_reconvergent_paths(i64 %val) {
+; CHECK-LABEL: test_reconvergent_paths:
+entry:
+  %l1a = add i64 %val, 10
+  %l1b = add i64 %val, 20
+  %l2a = mul i64 %l1a, 2
+  %l2b = mul i64 %l1b, 3
+  %l2c = mul i64 %l1a, 4
+  %l2d = mul i64 %l1b, 5
+  %l3a = add i64 %l2a, %l2b
+  %l3b = add i64 %l2c, %l2d
+  %merge = mul i64 %l3a, %l3b
+  ret i64 %merge
+}

for (auto *U : N->users()) {
auto UId = U->getNodeId();
if (UId > 0) {
if (UId > 0 && Visited.insert(U).second) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't the UId > 0 check be false the next time we try to visit the node?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I changed the code to this and the assert never failed.

      if (UId > 0) {
        assert(Visited.insert(U).second);

Copy link
Author

Choose a reason for hiding this comment

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

You're absolutely right and I should have tested this more carefully

I mistakenly thought nodes could be pushed multiple times before being
popped but InvalidateNodeId runs immediately when we first see the node
so UId becomes negative and the second check automatically fails

The visited set is redundant because the ID invalidation already prevents
revisits

Thanks for catching this - the assert test is a clean way to verify it

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.

3 participants