Re: [PATCH 2/3]rs6000: NFC use sext_hwi to replace ((v&0xf..f)^0x80..0) - 0x80..0

["Kewen.Lin" <linkw@linux.ibm.com>]

on 2022/12/1 13:35, Jiufu Guo wrote:
> Hi Kewen,
> 
> Thanks for your quick and insight review!
> 
> 在 12/1/22 1:17 PM, Kewen.Lin 写道:
>> Hi Jeff,
>>
>> on 2022/12/1 09:36, Jiufu Guo wrote:
>>> Hi,
>>>
>>> This patch just uses sext_hwi to replace the expression like:
>>> ((value & 0xf..f) ^ 0x80..0) - 0x80..0 for rs6000.cc and rs6000.md.
>>>
>>> Bootstrap & regtest pass on ppc64{,le}.
>>> Is this ok for trunk? 
>>
>> You didn't say it clearly but I guessed you have grepped in the whole
>> config/rs6000 directory, right?  I noticed there are still two places
>> using this kind of expression in function constant_generates_xxspltiw,
>> but I assumed it's intentional as their types are not HOST_WIDE_INT.
>>
>> gcc/config/rs6000/rs6000.cc:      short sign_h_word = ((h_word & 0xffff) ^ 0x8000) - 0x8000;
>> gcc/config/rs6000/rs6000.cc:  int sign_word = ((word & 0xffffffff) ^ 0x80000000) - 0x80000000;
>>
>> If so, could you state it clearly in commit log like "with type
>> signed/unsigned HOST_WIDE_INT" or similar?
>>
> Good question!
> 
> And as you said sext_hwi is more for "signed/unsigned HOST_WIDE_INT".
> For these two places, it seems sext_hwi is not needed actually!
> And I did see why these expressions are used, may be just an assignment
> is ok.

ah, I see.  I agree using the assignment is quite enough.  Could you
please also simplify them together?  Since they are with the form 
"((value & 0xf..f) ^ 0x80..0) - 0x80..0" too, and can be refactored
in a better way.  Thanks!

BR,
Kewen