Thanks your comments, Jeff and Robin > > Is the mulh case somehow common or critical? > Well, I would actually back up even further. What were the > circumstances that led to the mulh with a zero operand?
I think you both mentioned why should we add the mulh * 0 simplify. Unfortunately, I have no such a benchmark to explain the criticalness. We found there're some cases that exists in simplify_binary_operation in simplify-rtx.cc but not working for RISC-V backend. For example, - mult * 0 exists, but RISC-V has additional mulh * 0 - add + 0 / sub - 0 exists, but RISC-V has additional (madc + adc) + 0 - ... So we want to do some complement to make the simplify can cover more cases. That's the basic idea why we do these shortcut optimizations. > > However, adding new rtl expressions, especially generic ones that are > > useful for others and the respective optimizations is a tedious > > process as well. Still, just recently Roger Sayle added bitreverse > > and copysign. You can refer to his patch as well as the follow-up > > ones to get an idea of what would need to be done. > > ("Add RTX codes for BITREVERSE and COPYSIGN") Great advise. I'll have a check for the generic operations whether they can be implemented by this patch's style. It seems that we have to write specific pattern for the unspec relative insns, unfortunately. Thanks, Yanzhang > -----Original Message----- > From: Jeff Law <jeffreya...@gmail.com> > Sent: Saturday, July 29, 2023 7:07 AM > To: Robin Dapp <rdapp....@gmail.com>; Wang, Yanzhang > <yanzhang.w...@intel.com>; gcc-patches@gcc.gnu.org > Cc: juzhe.zh...@rivai.ai; kito.ch...@sifive.com; Li, Pan2 > <pan2...@intel.com> > Subject: Re: [PATCH v2] RISC-V: convert the mulh with 0 to mov 0 to the reg. > > > > On 7/28/23 06:31, Robin Dapp via Gcc-patches wrote: > >> This is a draft patch. I would like to explain it's hard to make the > >> simplify generic and ask for some help. > >> > >> There're 2 categories we need to optimize. > >> > >> - The op in optab such as div / 1. > >> - The unspec operation such as mulh * 0, (vadc+vmadc) + 0. > >> > >> Especially for the unspec operation, I found we need to write one by > >> one to match the special pattern. Seems there's no way to write a > >> generic pattern that will match mulh, (vadc+vmadc), sll... This way > >> is too complicated and not so elegant because need to write so much > >> md patterns. > >> > >> Do you have any ideas? > > > > Yes, it's cumbersome having to add the patterns individually and it > > would be nicer to have the middle end optimize for us. > > > > However, adding new rtl expressions, especially generic ones that are > > useful for others and the respective optimizations is a tedious > > process as well. Still, just recently Roger Sayle added bitreverse > > and copysign. You can refer to his patch as well as the follow-up > > ones to get an idea of what would need to be done. > > ("Add RTX codes for BITREVERSE and COPYSIGN") > > > > So if we have few patterns that are really performance critical (like > > for some benchmark) my take is to add them in a similar way you were > > proposing but I would advise against using this excessively. > > Is the mulh case somehow common or critical? > Well, I would actually back up even further. What were the > circumstances that led to the mulh with a zero operand? That would > tend to be an indicator of a problem earlier. Perhaps in the gimple > pipeline or the gimple->rtl conversion. I'd be a bit surprised to see a > const0_rtx propagate in during the RTL pipeline, I guess it's possible, but > I'd expect it to be relatively rare. > > The one case I could see happening would be cases from the builtin apis... > Of course one might call that user error ;-) > > > jeff