[PATCH] D130450: [JITLink] Relax zero-fill edge assertions.

Lang Hames via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 25 17:24:23 PDT 2022


lhames accepted this revision.
lhames added a comment.
This revision is now accepted and ready to land.

Otherwise LGTM.



================
Comment at: llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h:306-307
                Edge::AddendT Addend) {
-    assert(!isZeroFill() && "Adding edge to zero-fill block?");
+    assert((K < Edge::FirstRelocation || !isZeroFill()) &&
+           "Adding edge to zero-fill block?");
     Edges.push_back(Edge(K, Offset, Target, Addend));
----------------
Can this specifically check for K == Edge::KeepAlive for now? `Invalid` is also a non-relocation edge, but we wouldn't want it sneaking through here.


================
Comment at: llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.h:131-132
       LLVM_DEBUG(dbgs() << "    Applying fixups.\n");
-      assert((!B->isZeroFill() || B->edges_size() == 0) &&
-             "Edges in zero-fill block?");
       for (auto &E : B->edges()) {
----------------
We don't want to drop this entirely. I think it should be rewritten as:

```
assert((!B->isZeroFill() ||
        all_of(B->edges(), [](const Edge &E) { return E.getKind() == Edge::KeepAlive; })) &&
       "Non-KeepAlive edges in zero-fill block?");
```


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130450/new/

https://reviews.llvm.org/D130450



More information about the llvm-commits mailing list