[PATCH] D65506: [MachineScheduler] improve reuse of 'releaseNode'method

Thu Jan 2 02:07:17 PST 2020

fhahn added a comment.

In D65506#1800921 <https://reviews.llvm.org/D65506#1800921>, @DoktorC wrote:

> > It is because you are splitting the template declaration and definition into header and cpp. Usually, you need to add the explicit template instantiation in the cpp. However, I am not sure if it is a good idea to use template ... BTW, it compiles clean if with clang.
>
> You are totally right about explicit instantiation.
>
> Regarding the usage of a template argument, instead of a function parameter, lies in the fact that the scheduler is aware whether
>  a node is in the `Pending` queue or not, when calling `releaseNode`.
>  Indeed, such method is invoked in two occasions
>
> - a node is scheduled: successors/predecessors are released; thus, they're known to be not in the `Pending` queue.
> - moving nodes from pending to available state; the nodes are known to be in the `Pending` queue.
>
>   As a consequence, part of the operations perfomed by 'releaseNode' may be actively skipped, avoiding the overheads of control instructions. (Yes, modern architectures are endowed with branch predictors; but I do not think that such an argument saves us from correctly employ known invariants in the algorithms.)

FWIW I think we should trust the compiler to optimize this properly and passing it as parameter should be fine. Both versions should probably compile to the same code (although I don't have time to verify that right now)

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65506/new/

https://reviews.llvm.org/D65506