>> Yes, this is the emitted sequence, but the vsetvli mask is indeed
>> wrong.  Just got lucky there.  Or what else did you mean with
>> logically incorrect?
Oh, sorry. I didn't mean this patch logically incorrect.
I mean the MASK_ANY is logicall incorrect.
This patch is ok to me as long as you change MASK TAIL into MASK_UNDISTURBED.

Beside, V2 patch should change this:
emit_vlmax_masked_insn (unsigned icode, int op_num, rtx *ops)

change it into emit_vlmax_masked_mu_insn .


Thanks.


juzhe.zh...@rivai.ai
 
From: Robin Dapp
Date: 2023-05-25 20:32
To: juzhe.zh...@rivai.ai; gcc-patches; kito.cheng; palmer; jeffreyalaw
CC: rdapp.gcc
Subject: Re: [PATCH v2] RISC-V: Implement autovec abs, vneg, vnot.
> I think it's logically incorrect.  For ABS, you want:
> 
> operands[0] = operads[1] > 0 ? operands[1] :  (-operands[1])
> So you should do this following sequence:
> 
> vmslt v0,v1,0
> vneg v1,v1v0.t (should use Mask undisturbed)
 
Yes, this is the emitted sequence, but the vsetvli mask is indeed
wrong.  Just got lucky there.  Or what else did you mean with
logically incorrect?
 
> Here I see you set:
> e.set_policy (MASK_ANY); which is incorrect.
> You should use e.set_policy (MASK_UNDISTURBED); instead.> 
> Your testcases fail to catch this issue (you should create a testcase
> to catch this bug with this patch implementation.)
 
Added a regex to look for "ta,mu".
 
> You should not use RVV_UNOP+2. Instead, you should add an enum call
> RVV_UNOP_MU and replace it.
 
I was a bit weary of adding yet another, would rather have that
unified somehow, but well ;) Another time.  Adjusted locally.
 
 

Reply via email to