[PATCH] D31587: MachineScheduler/ScheduleDAG: Add support for getNode2Index

[Axel Davy via Phabricator via llvm-commits]

axeldavy added a comment.

In https://reviews.llvm.org/D31587#716821, @MatzeB wrote:

> - Choose a better function name!


getTopoIndexes ?

> - Now that the function is public you should also document some facts about it like what to use as index into the vector or the range of indizes coming out of it.

What about: 
/ / / Node2Index[SU->NodeNum] (when not boundary SU) gives the topological index in the topological sort of ScheduleDAGTopologicalSort.
/ / / The index is contained between 0 and SUnits.count()-1

> - Instead of returning `const vector<int>&` better use `ArrayRef<int>` as that gives less constraints on the internal implementation.

Ok

> - Is it necessary to return the array as a whole or would a simpler function suffice that takes a single `const SUnit&` and returns the index into the corresponding topological ordering?

No, I'd be fine with a function returning Node2Index[SU->NodeNum]. Do you think a function would be preferable ?
One worry is that if we write a function that takes SU and returns the index, it'd likely have to check for boundaries every call.


Repository:
  rL LLVM

https://reviews.llvm.org/D31587