On Tue, 16 May 2023 at 00:29, Richard Sandiford <richard.sandif...@arm.com> wrote: > > Prathamesh Kulkarni <prathamesh.kulka...@linaro.org> writes: > > Hi Richard, > > After committing the interleave+zip1 patch for vector initialization, > > it seems to regress the s32 case for this patch: > > > > int32x4_t f_s32(int32_t x) > > { > > return (int32x4_t) { x, x, x, 1 }; > > } > > > > code-gen: > > f_s32: > > movi v30.2s, 0x1 > > fmov s31, w0 > > dup v0.2s, v31.s[0] > > ins v30.s[0], v31.s[0] > > zip1 v0.4s, v0.4s, v30.4s > > ret > > > > instead of expected code-gen: > > f_s32: > > movi v31.2s, 0x1 > > dup v0.4s, w0 > > ins v0.s[3], v31.s[0] > > ret > > > > Cost for fallback sequence: 16 > > Cost for interleave and zip sequence: 12 > > > > For the above case, the cost for interleave+zip1 sequence is computed as: > > halves[0]: > > (set (reg:V2SI 96) > > (vec_duplicate:V2SI (reg/v:SI 93 [ x ]))) > > cost = 8 > > > > halves[1]: > > (set (reg:V2SI 97) > > (const_vector:V2SI [ > > (const_int 1 [0x1]) repeated x2 > > ])) > > (set (reg:V2SI 97) > > (vec_merge:V2SI (vec_duplicate:V2SI (reg/v:SI 93 [ x ])) > > (reg:V2SI 97) > > (const_int 1 [0x1]))) > > cost = 8 > > > > followed by: > > (set (reg:V4SI 95) > > (unspec:V4SI [ > > (subreg:V4SI (reg:V2SI 96) 0) > > (subreg:V4SI (reg:V2SI 97) 0) > > ] UNSPEC_ZIP1)) > > cost = 4 > > > > So the total cost becomes > > max(costs[0], costs[1]) + zip1_insn_cost > > = max(8, 8) + 4 > > = 12 > > > > While the fallback rtl sequence is: > > (set (reg:V4SI 95) > > (vec_duplicate:V4SI (reg/v:SI 93 [ x ]))) > > cost = 8 > > (set (reg:SI 98) > > (const_int 1 [0x1])) > > cost = 4 > > (set (reg:V4SI 95) > > (vec_merge:V4SI (vec_duplicate:V4SI (reg:SI 98)) > > (reg:V4SI 95) > > (const_int 8 [0x8]))) > > cost = 4 > > > > So total cost = 8 + 4 + 4 = 16, and we choose the interleave+zip1 sequence. > > > > I think the issue is probably that for the interleave+zip1 sequence we take > > max(costs[0], costs[1]) to reflect that both halves are interleaved, > > but for the fallback seq we use seq_cost, which assumes serial execution > > of insns in the sequence. > > For above fallback sequence, > > set (reg:V4SI 95) > > (vec_duplicate:V4SI (reg/v:SI 93 [ x ]))) > > and > > (set (reg:SI 98) > > (const_int 1 [0x1])) > > could be executed in parallel, which would make it's cost max(8, 4) + 4 = > > 12. > > Agreed. > > A good-enough substitute for this might be to ignore scalar moves > (for both alternatives) when costing for speed. Thanks for the suggestions. Just wondering for aarch64, if there's an easy way we can check if insn is a scalar move, similar to riscv's scalar_move_insn_p that checks if get_attr_type(insn) is TYPE_VIMOVXV or TYPE_VFMOVFV ? > > > I was wondering if we should we make cost for interleave+zip1 sequence > > more conservative > > by not taking max, but summing up costs[0] + costs[1] even for speed ? > > For this case, > > that would be 8 + 8 + 4 = 20. > > > > It generates the fallback sequence for other cases (s8, s16, s64) from > > the test-case. > > What does it do for the tests in the interleave+zip1 patch? If it doesn't > make a difference there then it sounds like we don't have enough tests. :) Oh right, the tests in interleave+zip1 patch only check for s16 case, sorry about that :/ Looking briefly at the code generated for s8, s32 and s64 case, (a) s8, and s16 seem to use same sequence for all cases. (b) s64 seems to use fallback sequence. (c) For vec-init-21.c, s8 and s16 cases prefer fallback sequence because costs are tied, while s32 case prefers interleave+zip1:
int32x4_t f_s32(int32_t x, int32_t y) { return (int32x4_t) { x, y, 1, 2 }; } Code-gen with interleave+zip1 sequence: f_s32: movi v31.2s, 0x1 movi v0.2s, 0x2 ins v31.s[0], w0 ins v0.s[0], w1 zip1 v0.4s, v31.4s, v0.4s ret Code-gen with fallback sequence: f_s32: adrp x2, .LC0 ldr q0, [x2, #:lo12:.LC0] ins v0.s[0], w0 ins v0.s[1], w1 ret Fallback sequence cost = 20 interleave+zip1 sequence cost = 12 I assume interleave+zip1 sequence is better in this case (chosen currently) ? I will send a patch to add cases for s8, s16 and s64 in a follow up patch soon. > > Summing is only conservative if the fallback sequence is somehow "safer". > But I don't think it is. Building an N-element vector from N scalars > can be done using N instructions in the fallback case and N+1 instructions > in the interleave+zip1 case. But the interleave+zip1 case is still > better (speedwise) for N==16. Ack, thanks. Should we also prefer interleave+zip1 when the costs are tied ? For eg, for the following case: int32x4_t f_s32(int32_t x) { return (int32x4_t) { x, 1, x, 1 }; } costs for both fallback and interleave+zip1 sequence = 12, and we currently choose fallback sequence. Code-gen: f_s32: movi v0.4s, 0x1 fmov s31, w0 ins v0.s[0], v31.s[0] ins v0.s[2], v31.s[0] ret while, if we choose interleave+zip1, code-gen is: f_s32: dup v31.2s, w0 movi v0.2s, 0x1 zip1 v0.4s, v31.4s, v0.4s ret I suppose the interleave+zip1 sequence is better in this case ? And more generally, if the costs are tied, would it be OK to prefer interleave+zip1 sequence since it will have parallel execution of two halves, which may not always be the case with fallback sequence ? Also, would it be OK to commit the above patch that addresses the issue with single constant case and xfail the s32 case for now ? Thanks, Prathamesh > > Thanks, > Richard