[PATCH] D147368: [LoongArch] Optimize bitwise and with immediates

[WÁNG Xuěruì via Phabricator via llvm-commits]

xen0n accepted this revision.
xen0n added a comment.

In D147368#4239678 <https://reviews.llvm.org/D147368#4239678>, @benshi001 wrote:

> In D147368#4239651 <https://reviews.llvm.org/D147368#4239651>, @xen0n wrote:
>
>> In D147368#4239647 <https://reviews.llvm.org/D147368#4239647>, @benshi001 wrote:
>>
>>> In D147368#4239614 <https://reviews.llvm.org/D147368#4239614>, @xen0n wrote:
>>>
>>>> LGTM from a cursory look, thanks! Although putting your informative replies into the code as comments would be even better, as others would see the rationale along with the code and not have to do archaeology themselves.
>>>
>>> Thanks for your suggestion. I have added some informative comments to both c++ code and the test code.
>>
>> Ah. I actually meant putting the justification for the "> 2 uses" (being conservative when faced with register pressure vs. instruction count blah blah), and it's actually fine to //not// comment //what// you did when the code is self-documenting. In general just document //why// and not //what// you do for a piece of code that may warrant such explanation.
>
> I have commented in the c++ code as follow
>
>   // Omit if the constant has more than 2 uses. This a conservative
>   // decision. Whether it is a win depends on the HW microarchitecture.
>   // However it should always be better for 1 and 2 uses.
>   if (CN->use_size() > 2)
>     return SDValue();

Looks good, thanks!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147368/new/

https://reviews.llvm.org/D147368