On Wed, Apr 5, 2023 at 3:53 PM <juzhe.zh...@rivai.ai> wrote: > > >> 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. > > The vsetvl demand fusion happens is not necessary "equivalent", instead, we > call it we will do demand fusion when they are "compatible". > And the fusion can happen between any vector insns including data movement > and actual computations. > > What is "compatible" ?? This definition is according to RVV ISA. > For example , For a vadd.vv need a vsetvl zero, 4, e32,m1,ta,ma. > and a vle.v need a vsetvl zero,4,e8,mf4,ta,ma. > > According to RVV ISA: > vadd.vv demand SEW = 32, LMUL = M1, AVL = 4 > vle.v demand RATIO = SEW/LMUL = 32, AVL = 4. > So after demand fusion, the demand becomes SEW = 32, LMUL = M1, AVL = 4. > Such vsetvl instruction is configured as this demand fusion, we call it > "compatible" > since we can find a common vsetvl VL/VTYPE status for both vadd.vv and vle.v > > However, what case is not "incompatible", same example, if the vadd.vv demand > SEW = 32. LMUL = MF2, > the vadd.vv is incompatible with vle.v. since we can't find a common VL/VTYPE > vsetvl instruction available > for both of them. > > We have local demand fusion which is Phase 1. Local demand fusion is doing > the fusion within a block > And also we have global demand fusion which is Phase 3. Global demand fusion > is doing across blocks. > > After Phase 1, each block has a single demand fusion. Phase 3 is doing global > demand fusion trying to > find the common VL/VTYPE status available for a bunch of blocks, and fuse > them into a single vsetvl. > So that we eliminate redundant vsetvli. > > Here is a example: > > bb 0: （vle.v demand RATIO = 32） > / \ > bb 1 bb 2 > / \ / \ > bb 3 bb 4 .... bb 5 > vadd vmul vdiv > (demand (demand (demand > sew = 8, sew = 8, sew = 8, > lmul = mf4) lmul = mf4, lmul = mf4, > tail policy = tu) mask policy = mu) > > So in this case, we should do the global demand fusion for bb 0, bb3, bb 4, > bb5. > since they are compatible according to RVV ISA. > The final demand info of vsetvl should be vsetvl e8,mf4,tu,mu and put it > in the bb0. Then we can avoid vsetvl in bb 3, 4, 5.
Just to throw in a comment here - I think you should present LCM with something it can identify as the same for compatible vsetvl and then it should just work? OTOH if "compatible" is not transitive that's not possible (but then I can't quickly make up an example where it wouldn't be). > >> We have more fusion rules according to RVV ISA. Phase 3 (Global demand > >> fusion) is > >> really important. > > >> 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. > > Sorry for bad comments in the codes. Currently, I am working on the first > patch > of auto-vectorization. After I sent the first patch of auto-vectorization for > you to > review. I would like to re-check all the comments and code style of VSETVL > PASS. > And refine them. > > > > > juzhe.zh...@rivai.ai > > From: Jeff Law > Date: 2023-04-05 21:05 > To: juzhe.zhong; gcc-patches > CC: kito.cheng; palmer > Subject: Re: [PATCH] RISC-V: Fix PR108279 > > > 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 >
