This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 2/2] Enable elimination of zext/sext
- From: Kugan <kugan dot vivekanandarajah at linaro dot org>
- To: gcc-patches at gcc dot gnu dot org
- Cc: Uros Bizjak <ubizjak at gmail dot com>, Richard Biener <richard dot guenther at gmail dot com>, Jakub Jelinek <jakub at redhat dot com>
- Date: Thu, 28 Aug 2014 17:29:16 +1000
- Subject: Re: [PATCH 2/2] Enable elimination of zext/sext
- Authentication-results: sourceware.org; auth=none
- References: <CAFULd4aAN3KnSHzNyLckHiNBj1wiwNvih9hCymP8fbWmiNNA9g at mail dot gmail dot com> <53FDD6CA dot 7000400 at linaro dot org> <53FEA5F0 dot 9070906 at linaro dot org> <alpine dot DEB dot 2 dot 11 dot 1408280840170 dot 1580 at laptop-mg dot saclay dot inria dot fr>
On 28/08/14 16:44, Marc Glisse wrote:
> On Thu, 28 Aug 2014, Kugan wrote:
>
>> On 27/08/14 23:02, Kugan wrote:
>>> On 27/08/14 20:01, Uros Bizjak wrote:
>>>> Hello!
>>>>
>>>>> 2014-08-07 Kugan Vivekanandarajah <kuganv@linaro.org>
>>>>>
>>>>> * calls.c (precompute_arguments): Check
>>>>> promoted_for_signed_and_unsigned_p and set the promoted mode.
>>>>> (promoted_for_signed_and_unsigned_p): New function.
>>>>> (expand_expr_real_1): Check promoted_for_signed_and_unsigned_p
>>>>> and set the promoted mode.
>>>>> * expr.h (promoted_for_signed_and_unsigned_p): New function
>>>>> definition.
>>>>> * cfgexpand.c (expand_gimple_stmt_1): Call emit_move_insn if
>>>>> SUBREG is promoted with SRP_SIGNED_AND_UNSIGNED.
>>>>
>>>> This patch regresses:
>>>>
>>>> Running target unix
>>>> FAIL: libgomp.fortran/simd7.f90 -O2 execution test
>>>> FAIL: libgomp.fortran/simd7.f90 -Os execution test
>>>>
>>>
>>> [snip]
>>>
>>>> When compiling this code, we have:
>>>>
>>>> lhs = _63
>>>> target = (subreg/s/v/u:SI (reg:DI 145 [ D.1694 ]) 0)
>>>> temp = (subreg:SI (reg:DI 540) 0)
>>>>
>>>> So, the code assumes that it is possible to copy (reg:DI 540) directly
>>>> to (reg:DI 154). However, this is not the case, since we still have
>>>> garbage in the top 32bits.
>>>>
>>>> Reverting the part above fixes the runtime failure, since (insn 599)
>>>> is now:
>>>>
>>>> (insn 599 598 0 (set (reg:DI 145 [ D.1694 ])
>>>> (zero_extend:DI (subreg:SI (reg:DI 540) 0))) -1
>>>> (nil))
>>>>
>>>> It looks to me that we have also to check the temp with
>>>> SUBREG_PROMOTED_*.
>>>
>>> Sorry for the breakage. I am looking into this now and I can reproduce
>>> it on qemu-alpha.
>>>
>>> I have noticed the following VRP data which is used in deciding this
>>> erroneous removal. It seems suspicious to me.
>>>
>>> _343: [2147483652, 2147483715]
>>> _344: [8, 134]
>>> _345: [8, 134]
>>>
>>> _343 = ivtmp.179_52 + 2147483645;
>>> _344 = _343 * 2;
>>> _345 = (integer(kind=4)) _344;
>>>
>>> Error comes from the third statement.
>>
>> In tree-vrp.c, in extract_range_from_binary_expr_1, there is a loss of
>> precision and the value_range is truncated. For the test-case provided
>> by Uros, it is
>>
>> _344 = _343 * 2;
>> [...,0x100000008], precision = 384
>> [...,0x100000086], precision = 384
>>
>> and it is converted to following when it goes from wide_int to tree.
>> [8, 134]
>
> Why do you believe that is wrong? Assuming _344 has a 32 bit type with
> wrapping overflow, this is just doing the wrapping modulo 2^32.
>
Indeed. I missed the TYPE_OVERFLOW_WRAPS check earlier. Thanks for
pointing me to that.
Kugan