On 4/2/23 16:40, juzhe.zh...@rivai.ai wrote:
This point is seletected not because LCM but by Phase 3 (VL/VTYPE demand
info backward fusion and propogation) which
is I introduced into VSETVL PASS to enhance LCM && improve vsetvl
instruction performance.
So fusion in this context is really about identifying cases where two
configuration settings are equivalent and you "fuse" them together.
Presumably this is only going to be possible when the vector insns are
just doing data movement rather than actual computations?
If my understanding is correct, I can kind of see why you're doing
fusion during phase 3. My sense is there's a better way, but I'm having
a bit of trouble working out the details of what that should be to
myself. In any event, revamping parts of the vsetvl insertion code
isn't the kind of thing we should be doing now.
WRT the actual patch. Please put a function comment on the
all_empty_predecessor_p method. Something like this perhaps?
/* Return TRUE if all the predecessors of CFG_BB have vsetvl
state that is valid or dirty, FALSE otherwise. */
That would seem to indicate the function is poorly named. Unless you're
using "empty" here to mean the state is valid or dirty. Either way it
seems like the function name ought to be improved.
The comments talk about bb1 being inside a loop. Nowhere do you check
that as far as I can tell.
When trying to understand what the patch is going I ran across this comment:
/* The local_dem vector insn_info of the block. */
vector_insn_info local_dem;
That comment really doesn't improve anything. "local_dem" is clearly
short-hand for something (local demand?), whatever it is, make it
clearer in the comment.
Jeff
