>> 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.

>> 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
 

Reply via email to