[PATCH] D99662: [AArch64] Add Machine InstCombiner patterns for FMUL indexed variant

Wed Apr 7 13:46:53 PDT 2021

asavonic added a comment.

In D99662#2674645 <https://reviews.llvm.org/D99662#2674645>, @SjoerdMeijer wrote:

> But the other thing I was just wondering, not that I mind these patterns here, but are we not expecting that the VDUP is sunk to its user? I think that's probably what I would expect, but don't know if that is a fair expectation.

If VDUP is moved into the same BB as its user, then VMUL_indexed is selected without these changes in Machine InstCombiner.
However, this does not happen for some cases (like the one in the LIT tests) with -O3 optimization; maybe I'm missing some option.

In D99662#2674812 <https://reviews.llvm.org/D99662#2674812>, @dmgreen wrote:

> I was going to put a comment on saying something similar, that this can be done in both ways. There is code in CGP that can sink dup's to users.

This code is in `AArch64TargetLowering::shouldSinkOperands`, right?

> But I think this makes a lot of sense though, due to the other benefits of machine combiner. Like the fact that it's shared across both ISels and can take things like critical path lengths into account.

So should we keep this patch?
I also worry that if we try to use shouldSinkOperands, we will have to handle all LLVM IR patterns that may
form a VDUP.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99662