[PATCH/expand] PR64011 Adjust bitsize when partial overflow happen for big-endian

[Jeff Law]

On 01/15/15 09:27, Jiong Wang wrote:
>
> On 15/01/15 03:51, Jeff Law wrote:
>> On 01/14/15 15:31, Jiong Wang wrote:
>>> agree, and I think the truncation is needed otherwise there may have ICE
>>> on some target.
>>>
>>> and I found current gcc LOCATION info is very good !
>>> have done an experimental hack on at "expand_assignment": 4931 where the
>>> tree is expanded,
>>> gcc could give quite useful & accurate warning based on tree LOCATION
>>> info.
>>>
>>> ./cc1 -O2 -mbig-endian pr48335-2.c
>>>
>>> pr48335-2.c: In function ‘f5’:
>>> pr48335-2.c:19:29: warning: overflow here !
>>>      ((U *)((char *) &s.d + 1))[3] = x;
>>>                                ^
>>>
>>> while we need to add warning at store_bit_field_using_insv where
>>> there is no accurate LOCATION info. but looks like it's acceptable?
>>>
>>> pr48335-2.c:19:33: warning: overflow here !
>>>      ((U *)((char *) &s.d + 1))[3] = x;
>>>                                    ^
>> Yes, I think we're on the right track now -- warn and truncate the the
>> insertion.
>>
>> I just scanned our set of warning flags to see if this would fit nicely
>> under any of the existing flags, and it doesn't.  I guess putting it
>> under -Wextra is probably best for now.
>>
>> I think the warning text should indicate that the statement will write
>> outside the bounds of the destination object or something along those
>> lines.
>
> thanks for suggestions. patch updated
>
> the warning message is as the following:
>
> ./cc1 -O2 pr48335-2.c -Wextra
>
> pr48335-2.c: In function ‘f5’:
> pr48335-2.c:19:33: warning: write of 16bit data outside the bound of
> destination object, data truncated into 8bit [-Wextra]
>     ((U *)((char *) &s.d + 1))[3] = x;
>                                   ^
> x86-64 bootstrap & regress ok.
>
> ok for trunk?
>
> one other thing is I found using of "insv" maybe sub-optimal in some
> situation.
> for example, even before my patch, modify type of U to char so there is
> no overflow.
>
> use BFI will cost one more fmov (cc1 -O2 test.c), maybe this is caused
> by bfi only
> support integer type, which may cause extra reg copy when there are both
> float & int type.
>
> // comment out the "if (maybe_expand_insn" in store_bit_field_using_insv
> // to fall back to other code path
> f5:
>          uxtb    w0, w0
>          stp     x29, x30, [sp, -16]!
>          fmov    s0, wzr
>          fmov    s1, w0
>          add     x29, sp, 0
>          bl      bar
>          ldp     x29, x30, [sp], 16
>          ret
>
> // BFI version
> f5:
>          fmov    s0, wzr
>          stp     x29, x30, [sp, -16]!
>          add     x29, sp, 0
>          fmov    w1, s0
>          bfi     w1, w0, 0, 8
>          fmov    s1, w1
>          bl      bar
>          ldp     x29, x30, [sp], 16
>          ret
>
>
> Thanks.
>
> gcc/
>      PR64011
>      * expmed.c (store_bit_field_using_insv): Warn and truncate bitsize
> when there is partial overflow.
OK for the trunk.   Please install.

Thanks,
Jeff