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 <kug...@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
