Re: Re: [PATCH] RISC-V: Support vfwnmacc/vfwmsac/vfwnmsac combine lowering

["juzhe.zhong@rivai.ai" <juzhe.zhong@rivai.ai>]

[-- Attachment #1: Type: text/plain, Size: 2212 bytes --]

>> I've triple checked this already.
You mean you still didn't see vfwmul.vv ?

That's odd. Let's wait for kito or Robin test this patch.
Then, I believe they will know what I am saying.

>> I would strongly suggest looking at a dependency height reduction
>> pattern if you want to optimize that code further.
I did it long time ago. Turns out it's better to do that on Combine PASS in both GCC and LLVM.

Never mind, I always have this implementation in my downstream and won't affect my downstream GCC maintainment.
It's ok that this patch is not approved since I can get the perfect codegen in my downstream. 

Thanks.


juzhe.zhong@rivai.ai
 
From: Jeff Law
Date: 2023-06-30 09:26
To: juzhe.zhong@rivai.ai; gcc-patches
CC: kito.cheng; Kito.cheng; palmer; palmer; Robin Dapp
Subject: Re: [PATCH] RISC-V: Support vfwnmacc/vfwmsac/vfwnmsac combine lowering
 
 
On 6/29/23 19:14, juzhe.zhong@rivai.ai wrote:
> No, reduction patterns won't help.
> As I said in vfwmul patch. You should make sure your environment is 
> working then try again.
I've triple checked this already.
 
I checked it again and your patch does not impact behavior, nor should 
it.   I checked it on top of these trunk commits:
 
14bfda6084eaca07c842566a34316974907958e2
e714af12e3bee0032d8d226f87d92c9bc46f0269
 
I checked it with the code from the godbolt links you suggested with the 
options shown in those links.
 
More importantly, your explanation of what the pattern is supposed to do 
shows a misunderstanding of what combine's capabilities actually are.  A 
bridge or intermediate pattern is not needed here, combine can 
substitute multiple sources in combination attempts as can be clearly 
seen from the dump fragments I posted.
 
The only reason I didn't reject the patch at the outset was the 
possibility that maybe we were trying to combine more than 4 
instructions or that possibility something about the number of operands, 
unspecs, whatever were getting in the way.
 
This patch is not needed and does not affect code generation.
 
I would strongly suggest looking at a dependency height reduction 
pattern if you want to optimize that code further.
 
Jeff