[PATCH] D110561: [AArch64] Alter arith-cbz-fusion to fuse between pairs register instructions

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

In D110561#3027953 <https://reviews.llvm.org/D110561#3027953>, @MatzeB wrote:

>> This code adds an SDep::Artificial dependency between the instruction and ExitSU:
>
> But ExitSU is an artificial node not an to mark the end of the schedule graph. I don't think it makes sense trying to macro-fuse ExitSU with anything.
>
> The thing that is odd with this diff is that it adds extra checks to one particular rule (`isArithmeticCbzPair`) while it seems to me that if this is required, then all the other rules are problematic in the same sense; so we'd better understand why and find a general fix.

ExitSU is the `CBNZW` or `Bcc` according to the debug logs I'm looking at.

In D110561#3027959 <https://reviews.llvm.org/D110561#3027959>, @MatzeB wrote:

> In D110561#3027811 <https://reviews.llvm.org/D110561#3027811>, @dmgreen wrote:
>
>> This code adds an SDep::Artificial dependency between the instruction and ExitSU:
>> https://github.com/llvm/llvm-project/blob/20c02807333a47000879e0f673cdf2d6b07148dd/llvm/lib/CodeGen/ScheduleDAGInstrs.cpp#L876
>
> Have you tried what happens when we ignore `Dep.isArtificial()` in MacroFusion.cpp?

Yeah, that is what I meant by "Skipping Artificial edges in the same way we skip Weak edges will cause some of the other fusions to no longer correctly apply, as far as I can see. Like the arithmetic-bcc fusion and some of the X86 fusions."  There are a number of failing test cases I think because the dependencies between a `$xzr = SUBSXri $x2, 0, 0, implicit-def $nzcv` and a `Bcc 1, %bb.1, implicit killed $nzcv` is an artificial dependency, potentially through the physical nzcv register.

I don't have a strong need for this patch. I just noticed it started failing when changing the default schedule for -mcpu=generic. I think it can happen whenever another edge is added between nodes in the scheduling DAG, in this case between instructions with latency > 1 and ExitSU. It looks like the only other fusion that will be between an instruction and ExitSU will be isArithmeticBccPair, where the dependency goes through NSVZ.


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

https://reviews.llvm.org/D110561