[PATCH] D35228: [TableGen] Add support for instruction clusters

Sun Jul 16 05:53:18 PDT 2017

hfinkel added a comment.

In https://reviews.llvm.org/D35228#810596, @fhahn wrote:

> In https://reviews.llvm.org/D35228#807888, @hfinkel wrote:
>
> > I agree, it does not spread the information out over multiple files. However, the fact that the clusters are just specified by numbers makes the implementation less self-explanatory (i.e. it is really not clear what the numbers mean in the model files). I suggest that you add separate tablegen classes that represent the clusters so that you can refer to the clusters by name in the scheduling definitions.
>
>
> Agreed, putting the fusion information in the scheduling model makes sense conceptually, as it's a scheduling problem.
>
> With spreading out I meant the changes to the machine scheduler related files (but that's mostly just passing through the cluster info) and that we need to add fusion entries to all relevant machine models , whereas before there was a single file describing which instructions should be fused.
>
> For AArch64, there might be 2 potential problems:
>
> - Currently machine models are shared by different CPUs. What if one CPU supports fusing more instructions than the other? I'm not sure if that would be a practical issue at the moment, but it may be worth considering.
> - For fusing some instructions, more complicated constraints are used, e.g. CBZ and some arithmetic instructions are only fused if no shifted register is used [1] or MOVZWi should only be fused with MOVKWi, if the immediate used with MOVKWi  == 16 [2]. Could such constraints be expressed in the machine model?

Maybe the TableGen model could support optional C++ predicates (much like we do for isel). We could also have a target callback that can override the td-file clustering for special cases.

> [1] https://github.com/llvm-mirror/llvm/blob/master/lib/Target/AArch64/AArch64MacroFusion.cpp#L70
>  [2] https://github.com/llvm-mirror/llvm/blob/master/lib/Target/AArch64/AArch64MacroFusion.cpp#L140

https://reviews.llvm.org/D35228