Hi Richi, On 2020/9/7 19:57, Richard Biener wrote: > + if (TREE_CODE (to) == ARRAY_REF) > + { > + tree op0 = TREE_OPERAND (to, 0); > + if (TREE_CODE (op0) == VIEW_CONVERT_EXPR > + && expand_view_convert_to_vec_set (to, from, to_rtx)) > + { > + pop_temp_slots (); > + return; > + } > + } > > you're placing this at an awkward spot IMHO, after to_rtx expansion > but disregading parts of it and compensating just with 'to' matching. > Is the pieces (offset, bitpos) really too awkward to work with for > matching? > > Because as written you'll miscompile > > struct X { _vector signed int v; _vector singed int u; } x; > > test(int i, int a) > { > x.u[i] = a; > } > > as I think you'll end up assigning to x.v.
Thanks for pointing out, this case will be a problem for the patch. I checked with optimize_bitfield_assignment_op, it will return very early as the mode1 is not VOIDmode, and this is actually not "FIELD op= VAL" operation? To be honest, I am not quite familiar with this part of code, I put the new function expand_view_convert_to_vec_set just after to_rtx expansion because adjust_address will change the V4SImode memory to SImode memory, but I need keep target to_rtx V4SImode to save the vector after calling rs6000_vector_set_var, so seems paradoxical here? p to_rtx $264 = (rtx_def *) (mem/c:V4SI (reg/f:DI 112 virtual-stack-vars) [1 D.3186+0 S16 A128]) => to_rtx = adjust_address (to_rtx, mode1, 0); p to_rtx $265 = (rtx_def *) (mem/c:SI (reg/f:DI 112 virtual-stack-vars) [1 D.3186+0 S4 A128]) > > Are we just interested in the case were we store to a > pseudo or also when the destination is memory? I guess > only when it's a pseudo - correct? In that case > handling this all in optimize_bitfield_assignment_op > is probably the best thing to try. > > Note we possibly refrain from assigning a pseudo to > such vector because we see a variable array-ref to it. Seems not only pseudo, for example "v = vec_insert (i, v, n);" the vector variable will be store to stack first, then [r112:DI] is a memory here to be processed. So the patch loads it from stack(insn #10) to temp vector register first, and store to stack again(insn #24) after rs6000_vector_set_var. optimized: D.3185 = v_3(D); _1 = n_5(D) & 3; VIEW_CONVERT_EXPR<int[4]>(D.3185)[_1] = i_6(D); v_8 = D.3185; return v_8; => expand without the patch: 2: r119:V4SI=%2:V4SI 3: r120:DI=%5:DI 4: r121:DI=%6:DI 5: NOTE_INSN_FUNCTION_BEG 8: [r112:DI]=r119:V4SI 9: r122:DI=r121:DI&0x3 10: r123:DI=r122:DI<<0x2 11: r124:DI=r112:DI+r123:DI 12: [r124:DI]=r120:DI#0 13: r126:V4SI=[r112:DI] 14: r118:V4SI=r126:V4SI 18: %2:V4SI=r118:V4SI 19: use %2:V4SI => expand with the patch (replace #9~#12 to #10~#24): 2: r119:V4SI=%2:V4SI 3: r120:DI=%5:DI 4: r121:DI=%6:DI 5: NOTE_INSN_FUNCTION_BEG 8: [r112:DI]=r119:V4SI 9: r122:DI=r121:DI&0x3 10: r123:V4SI=[r112:DI] // load from stack 11: {r125:SI=0x3-r122:DI#0;clobber ca:SI;} 12: r125:SI=r125:SI<<0x2 13: {r125:SI=0x14-r125:SI;clobber ca:SI;} 14: r128:DI=unspec[`*.LC0',%2:DI] 47 REG_EQUAL `*.LC0' 15: r127:V2DI=[r128:DI] REG_EQUAL const_vector 16: r126:V16QI=r127:V2DI#0 17: r129:V16QI=unspec[r120:DI#0] 61 18: r130:V16QI=unspec[r125:SI] 151 19: r131:V16QI=unspec[r129:V16QI,r129:V16QI,r130:V16QI] 232 20: r132:V16QI=unspec[r126:V16QI,r126:V16QI,r130:V16QI] 232 21: r124:V16QI=r123:V4SI#0 22: r124:V16QI={(r132:V16QI!=const_vector)?r131:V16QI:r124:V16QI} 23: r123:V4SI=r124:V16QI#0 24: [r112:DI]=r123:V4SI // store to stack. 25: r134:V4SI=[r112:DI] 26: r118:V4SI=r134:V4SI 30: %2:V4SI=r118:V4SI 31: use %2:V4SI Thanks, Xionghu