Just tiniest nits (that obviously don't warrant a new version) >> + /* Remove the previous initialiation of IV_INCREMENT to VARYING.
Initialization. >> + Set *DR_INCREMENT to the amount that should be added to pointer base >> address >> + to get to the next iteration's base address. >> + Set *DR_BUMP to the amount that should be added to the base >> + address after each copy of the vectorized statement in a grouped read. >> + Set *VEC_OFFSET to an invariant offset vector in which element I has the >> + value I * DR_STEP / SCALE. */ >> >> static void >> vect_get_strided_load_store_ops (stmt_vec_info stmt_info, slp_tree node, >> tree vectype, tree offset_vectype, >> loop_vec_info loop_vinfo, >> gimple_stmt_iterator *gsi, >> - tree *dataref_bump, tree *vec_offset, >> - vec_loop_lens *loop_lens) >> + tree *dr_step, tree *dr_bump, This is still dr_step here vs dr_increment in the comment :) >> +#define LOOP_VINFO_IV_INCREMENT_INVARIANT_P(L)\ >> + (!(L) || !LOOP_VINFO_USING_SELECT_VL_P (L)) > > I considered making a comment here since it's weird that loop_vec_info > can be null when calling a LOOP_VINFO_ macro. It looks like that's because > it's also called in areas where we're doing BB vectorization. > > But we don't seem to have a VINFO_ macro group. So I was considering > asking for you to drop the LOOP_ prefix and check is_a <..> () instead of > NULL check to distinguish between a NULL value being passed and it not > being a loop. (and so when vinfo is available pass it instead of loop_vinfo). > > But I don't think it makes much of a difference here. An actual NULL would > fail elsewhere and so the vect changes are OK with the condition above > corrected. > > But I can't approve the RISCV test update (even though it looks like trivial > scheduling changes). The riscv test is OK (arguably it shouldn't even test for the exact sequence). -- Regards Robin
