[llvm] 896dd32 - [ORC] Fix block dependence calculation in ObjectLinkingLayer.

[Lang Hames via llvm-commits]

Author: Lang Hames
Date: 2024-06-26T13:00:41+10:00
New Revision: 896dd322afcc1cf5dc4fa7375dedd55b59001eb4

URL: https://github.com/llvm/llvm-project/commit/896dd322afcc1cf5dc4fa7375dedd55b59001eb4
DIFF: https://github.com/llvm/llvm-project/commit/896dd322afcc1cf5dc4fa7375dedd55b59001eb4.diff

LOG: [ORC] Fix block dependence calculation in ObjectLinkingLayer.

This fixes a bug in ObjectLinkingLayer::computeBlockNonLocalDeps: The worklist
needs to be built *after* all immediate dependencies / dependants are recorded,
rather than trying to populate it as part of the same loop. (Trying to do the
latter causes us to miss some blocks that should have been included in the
worklist).

This fixes a bug discovered by @Sahil123 on discord during work on
out-of-process execution support in the clang-repl.

No testcase yet. This *might* be testable with a unit test and a custom
JITLinkContext but I believe some aspects of the algorithm depend on memory
layout. I'll need to investigate that. Alternatively we could add llvm-jitlink
testcases that exercise concurrent linking (and should probably do that anyway).
Either option will require some investment and I don't want to hold this fix up
in the mean time.

Added: 
    

Modified: 
    llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp b/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
index 57ade94767865..a66c40ddb6877 100644
--- a/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
@@ -626,9 +626,11 @@ class ObjectLinkingLayerJITLinkContext final : public JITLinkContext {
           }
         }
       }
+    }
 
-      // If this node has both dependants and dependencies then add it to the
-      // worklist to propagate the dependencies to the dependants.
+    // Add blocks with both dependants and dependencies to the worklist to
+    // propagate dependencies to dependants.
+    for (auto &[B, BI] : BlockInfos) {
       if (!BI.Dependants.empty() && !BI.Dependencies.empty())
         WorkList.push_back(B);
     }