Re: [PATCH 1/1] Fix bit-position comparison

[Richard Sandiford <richard.sandiford@arm.com>]

Richard Biener <rguenther@suse.de> writes:
> On Wed, 27 Jul 2022, juzhe.zhong@rivai.ai wrote:
>
>> From: zhongjuzhe <juzhe.zhong@rivai.ai>
>> 
>> gcc/ChangeLog:
>> 
>>         * expr.cc (expand_assignment): Change GET_MODE_PRECISION to GET_MODE_BITSIZE
>> 
>> ---
>>  gcc/expr.cc | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/gcc/expr.cc b/gcc/expr.cc
>> index 80bb1b8a4c5..ac2b3c07df6 100644
>> --- a/gcc/expr.cc
>> +++ b/gcc/expr.cc
>> @@ -5574,7 +5574,7 @@ expand_assignment (tree to, tree from, bool nontemporal)
>>  	 code contains an out-of-bounds access to a small array.  */
>>        if (!MEM_P (to_rtx)
>>  	  && GET_MODE (to_rtx) != BLKmode
>> -	  && known_ge (bitpos, GET_MODE_PRECISION (GET_MODE (to_rtx))))
>> +	  && known_ge (bitpos, GET_MODE_BITSIZE (GET_MODE (to_rtx))))
>
> I think this has the chance to go wrong with regard to endianess.
> Consider to_rtx with 32bit mode size but 12bit mode precision.  bitpos
> is relative to the start of to_rtx as if it were in memory and bitsize
> determines the contiguous region affected.  But since we are actually
> storing into a non-memory endianess comes into play.

Not sure I follow the code well enough to comment, but: this condition
is saying when we can drop the assignment on the floor and just expand
the rhs (for side effects).  From that point of view, using bitsize
should be more conservative than using precision, since bitsize is
always >= precision.

Thanks,
Richard

>
> So no, I don't think the patch is correct, it would be way more
> complicated to get the desired improvement.
>
> Richard.
>
>>  	{
>>  	  expand_normal (from);
>>  	  result = NULL;
>>