[PATCH] D149169: [RISCV] Cost constant materialization of vectors in phis

[Luke Lau via Phabricator via llvm-commits]

luke added inline comments.


================
Comment at: llvm/test/Analysis/CostModel/RISCV/rvv-phi-const.ll:8
 ; CHECK-NEXT:  Cost Model: Found an estimated cost of 1 for instruction: br label %d
-; CHECK-NEXT:  Cost Model: Found an estimated cost of 1 for instruction: %x = phi <2 x i8> [ <i8 1, i8 -1>, %a ], [ <i8 -1, i8 1>, %b ]
+; CHECK-NEXT:  Cost Model: Found an estimated cost of 3 for instruction: %x = phi <2 x i8> [ <i8 1, i8 -1>, %a ], [ <i8 -1, i8 1>, %b ]
 ; CHECK-NEXT:  Cost Model: Found an estimated cost of 1 for instruction: ret <2 x i8> %x
----------------
ABataev wrote:
> luke wrote:
> > ABataev wrote:
> > > luke wrote:
> > > > The generated code:
> > > > ```
> > > > f:                                      # @f
> > > > 	.cfi_startproc
> > > > # %bb.0:
> > > > 	vsetivli	zero, 2, e8, mf8, ta, ma
> > > > 	vid.v	v8
> > > > 	andi	a0, a0, 1
> > > > 	vadd.vv	v8, v8, v8
> > > > 	beqz	a0, .LBB0_2
> > > > # %bb.1:
> > > > 	vrsub.vi	v8, v8, 1
> > > > 	ret
> > > > .LBB0_2:                                # %b
> > > > 	vadd.vi	v8, v8, -1
> > > > 	ret
> > > > ```
> > > I have one concern here about loops. In the loop we need to account for the materialization cost only once, if the input constant is a live-in. So for such values better to consider it free, I assume.
> > That's a good point. The cost of constants in stores must also be currently affected by this too right?
> Looks so. But it requires some significant rework of the cost model, I assume. For phi's you can try just to check if there is a loop in the graph (with the small enough limit), without actual analysis/use of the Loop objects.
Yes, and it it looks like arithmetic instructions are also affected, not just on RISC-V but on at least AArch64 too.
If this is a pervasive problem that already exists across the entire cost model then perhaps it would be best to stay consistent and include materialisation in getPHICost, but do some sort of "temporary" workaround in SLP whenever it's in a loop, as you suggest.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149169