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

[Matthias Braun via Phabricator via llvm-commits]

MatzeB added a comment.

In https://reviews.llvm.org/D31587#717117, @axeldavy wrote:

> In https://reviews.llvm.org/D31587#716821, @MatzeB wrote:
>
> > - Choose a better function name!
>
>
> getTopoIndexes ?


that sounds better.

>> - 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

Maybe "Maps an SUnit node number to indexes of a topological ordering. The index is between 0 and SUnits.count()-1. Boundary SUnits are not mapped."

>> - 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