[PATCH] D139790: [VPlan] Consider all recipes in replicate blocks as sink candidates.

Sat Dec 31 15:12:02 PST 2022

Ayal added inline comments.

================
Comment at: llvm/test/Transforms/LoopVectorize/vplan-sink-scalars-and-merge.ll:38
+; CHECK-NEXT:     vp<[[STEPS:%.+]]> = SCALAR-STEPS vp<[[CAN_IV]]>, ir<1>
 ; CHECK-NEXT:     REPLICATE ir<%gep.b> = getelementptr ir<@b>, ir<0>, vp<[[STEPS]]>
 ; CHECK-NEXT:     REPLICATE ir<%lv.b> = load ir<%gep.b>
----------------
fhahn wrote:
> Ayal wrote:
> > fhahn wrote:
> > > Ayal wrote:
> > > > The iterative scan up use-def chains stops at GEPs?
> > > At the moment only replicate & scalar-steps recipe can be sunk.
> > Right, but curious why the use-def scan failed to sink vp<[[STEPS]]> used by sunk REPLICATEs of GEPS while scanning all recipes in replicate block succeeded. Is this due to the now iterative nature of applying sinkScalarOperands()?
> Ah right! I had another look and the issue was that operands of instructions that do not need sinking aren't added to the worklist. Should be fixed by aa2414729ebb
Hmm, that may have been intentional - to prevent revisiting same candidates multiple times during a scan up use-def chains. But that assumed operands that do not need sinking were sunk in the "current" scan - when activated only once, starting with a single seed per replicate region. Whereas now the first scan starts with a single seed but repeated iterative scans should indeed consider all / more-than-one?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139790