[PATCH] D50680: [SDAG] Remove the reliance on MI's allocation strategy for `MachineMemOperand` pointers attached to `MachineSDNodes` and instead have the `SelectionDAG` fully manage the memory for this array.

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/SelectionDAGNodes.h:2256
 
-  /// Memory reference descriptions for this instruction.
-  mmo_iterator MemRefs = nullptr;
-  mmo_iterator MemRefsEnd = nullptr;
+  PointerUnion<MachineMemOperand *, MachineMemOperand **> MemRefs = {};
+
----------------
chandlerc wrote:
> hfinkel wrote:
> > Should we use TinyPtrVector here instead of making this PointerUnion and keeping NumMemRefs below?
> We can't. I tried so hard to do this. It was terrible.
> 
> In a word, `MorphNodeTo`.
> 
> We don't actually run constructors or destructors reliably for any SDNode subclasses. We allocate enough memory and with enough alignment for any of the subclasses, and then we just "switch" the subclass by writing to `NodeType`. Yay.
> 
> We have manual code in `MorphNodeTo` to "clean up" and "initialize" the members as needed.
> 
> I tried fixing this, and it is terrible. Nothing works.
> 
> We essentially *have* to allocate any extra storage off the DAG's allocator so it gets freed for us, and we have to use POD-ish data types here that we can update with a simple store regardless of what uninitialized garbage memory is left there due to `MorphNodeTo`.
> 
> Sorry I couldn't find anything less awful than this.
Makes sense. Can you please add this as a comment?

Otherwise, I'm happy with this patch.


Repository:
  rL LLVM

https://reviews.llvm.org/D50680