[PATCH] D70986: [DDG] Data Dependence Graph - Ordinals

[Bardia Mahjour via Phabricator via llvm-commits]

bmahjour marked 2 inline comments as done.
bmahjour added a comment.

In D70986#1771141 <https://reviews.llvm.org/D70986#1771141>, @Meinersbur wrote:

> I do not understand why the ordinals are necessary. For deterministic behavior, the deterministic iteration order over blocks and instructions should already ensure that (unless you are iterating over DenseMaps, which does not seem the case and is easily fixed). If for correctness, do you have an illustrative example when this would be critical?


The nodes in a pi-block cannot be topologically sorted since they form a cycle. The ordinals help us put them in their original program order so as to preserve correctness. For example consider this case:

  for (int i = 0; i < n-1; i++) {
    A[i] = B[i];
    B[i+1] = A[i];
  }

There will be a pi-block consisting of all load and stores in the loop, and they cannot be sorted in the order data dependence. We do not want to inadvertently change it to:

  for (int i = 0; i < n-1; i++) {
    B[i+1] = A[i];
    A[i] = B[i];
  }



================
Comment at: llvm/lib/Analysis/DependenceGraphBuilder.cpp:57
       IMap.insert(std::make_pair(&I, &NewNode));
+      NodeOrdinalMap.insert(std::make_pair(&NewNode, getOrdinal(I)));
       ++TotalFineGrainedNodes;
----------------
Meinersbur wrote:
> Did you consider string the node ordinal in the Node directly instead of using a lookup table?
Ordinal numbers take up space and are only needed during construction of the graph. The users of DDG don't need the ordinal, so I decided to save space and use the maps instead. The maps get cleared as soon as we are done with pi-block creation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70986