This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Cost constant materialization of vectors in phis
Changes PlannedPublic

Authored by luke on Apr 25 2023, 10:22 AM.

Details

Summary

If a phi has a constant operand it will need to be materialized at some
point, so account for this cost by reusing the logic used for store
immediates.
This should eventually be improved to also handle scalar constants.

Diff Detail

Event Timeline

luke created this revision.Apr 25 2023, 10:22 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 25 2023, 10:22 AM
luke requested review of this revision.Apr 25 2023, 10:22 AM
luke added inline comments.
llvm/test/Analysis/CostModel/RISCV/rvv-phi-const.ll
9

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
ABataev added inline comments.Apr 26 2023, 9:24 AM
llvm/test/Analysis/CostModel/RISCV/rvv-phi-const.ll
9

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.

luke added inline comments.Apr 27 2023, 2:58 AM
llvm/test/Analysis/CostModel/RISCV/rvv-phi-const.ll
9

That's a good point. The cost of constants in stores must also be currently affected by this too right?

ABataev added inline comments.Apr 27 2023, 4:20 AM
llvm/test/Analysis/CostModel/RISCV/rvv-phi-const.ll
9

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.

luke added inline comments.Apr 27 2023, 8:00 AM
llvm/test/Analysis/CostModel/RISCV/rvv-phi-const.ll
9

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.

ABataev added inline comments.Apr 27 2023, 9:06 AM
llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
1428

const OperandValueInfo &OpInfo

luke updated this revision to Diff 517629.Apr 27 2023, 10:17 AM

Address comment and rebase

luke planned changes to this revision.Jun 29 2023, 5:53 AM

After discussing offline with @reames, we're not sure if this is the most accurate way to model constant costs in phi nodes. Moving it off the review queue for the meantime

evandro removed a subscriber: evandro.Jun 30 2023, 3:44 PM