[PATCH] D29295: Move core RDF files from lib/Target/Hexagon to CodeGen

[Matthias Braun via Phabricator via llvm-commits]

MatzeB added a comment.

My intuition would be that needing/computing a full dataflow graph that late in the compilation process is a bad sign. Having said that this probably won't go away for Hexagon and sharing code between targets is a good idea. Do you know of any plans to use this in other targets?

Some random nitpicks following but I didn't really read the whole patch yet.



================
Comment at: include/llvm/CodeGen/RDFGraph.h:10
+//
+// Target-independent, SSA-based data flow graph for register data flow (RDF)
+// for a non-SSA program representation (e.g. post-RA machine code).
----------------
should be `/// \file`


================
Comment at: include/llvm/CodeGen/RDFGraph.h:278
+      KindMask      = 0x0007 << 2,
+      Def           = 0x0001 << 2,  // 001
+      Use           = 0x0002 << 2,  // 010
----------------
I usually declare bitsets like this:

```
enum MyBitSet {
   A = 1u << 0,
   B = 1u << 1,
   C = 1u << 2,
   // ...
   // or 1u << FirstBit, 1u << FirstBit+1, if you cannot start at 1
   Mask = 0xf // Though I usually avoid masks and rather try to pack stuff into separate bitfield
              // members instead of putting them all in a uint16_t
};
```
which makes it more obvious what is happening.


================
Comment at: include/llvm/CodeGen/RDFGraph.h:360
+
+  // Fast memory allocation and translation between node id and node address.
+  // This is really the same idea as the one underlying the "bump pointer
----------------
Use doxygen comments.


================
Comment at: lib/CodeGen/RDFLiveness.cpp:19
+//
+// Dibyendu Das, Ramakrishna Upadrasta, Benoit Dupont de Dinechin.
+// "Efficient Liveness Computation Using Merge Sets and DJ-Graphs."
----------------
There is an alternative algorithm floating around, for example https://reviews.llvm.org/D28934

Which I usually prefer because it is simpler and doesn't require dominance tree construction. But I may be biased here :)


Repository:
  rL LLVM

https://reviews.llvm.org/D29295