craig.topper marked 2 inline comments as done. craig.topper added inline comments.
================ Comment at: llvm/lib/Target/X86/ImmutableGraph.h:285 + std::unique_ptr<Edge[]> Edges; + size_type EdgesSize; +}; ---------------- sconstab wrote: > @craig.topper It now occurs to me that these fields should probably be > reordered to: > ``` > std::unique_ptr<Node[]> Nodes; > std::unique_ptr<Edge[]> Edges; > size_type NodesSize; > size_type EdgesSize; > ``` > The current ordering will cause internal fragmentation. > > Old ordering: > ``` > static_assert(sizeof(ImmutableGraph<T, V>) == 32); > ``` > New ordering: > ``` > static_assert(sizeof(ImmutableGraph<T, V>) == 24); > ``` > With vectors instead of arrays: > ``` > static_assert(sizeof(ImmutableGraph<T, V>) == 48); > ``` I noticed that too. I just didn't focus on it since we only ever one in memory at a time. I'll change in my next update. ================ Comment at: llvm/lib/Target/X86/ImmutableGraph.h:307 +public: + using NodeRef = size_type; + ---------------- sconstab wrote: > Just noticed that `ImmutableGraphBuilder` and `ImmutableGraph` have > non-identical types called `NodeRef`. Suggest renaming this one to > `BuilderNodeRef`. NodeRef is in the Traits class not the ImmutableGraph, but I will rename the builder one. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75936/new/ https://reviews.llvm.org/D75936 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
