2012/2/9 Richard Guenther <richard.guent...@gmail.com>:
> On Thu, Feb 9, 2012 at 3:41 PM, Richard Guenther
> <richard.guent...@gmail.com> wrote:
>> On Thu, Feb 9, 2012 at 2:48 PM, Kai Tietz <ktiet...@googlemail.com> wrote:
>>> 2012/2/9 Richard Guenther <richard.guent...@gmail.com>:
>>>> On Thu, Feb 9, 2012 at 11:53 AM, Richard Guenther
>>>> <richard.guent...@gmail.com> wrote:
>>>>> On Thu, Feb 9, 2012 at 11:29 AM, Richard Guenther
>>>>> <richard.guent...@gmail.com> wrote:
>>>>>> On Wed, Feb 8, 2012 at 10:25 PM, Kai Tietz <ktiet...@googlemail.com>
>>>>>> wrote:
>>>>>>> 2012/1/11 Richard Guenther <richard.guent...@gmail.com>:
>>>>>>>>
>>>>>>>> count despite being declared volatile and only loaded once in the
>>>>>>>> source
>>>>>>>> is loaded twice in gimple. If it were a HW register which destroys the
>>>>>>>> device after the 2nd load without an intervening store you'd wrecked
>>>>>>>> the device ;)
>>>>>>>>
>>>>>>>> Richard.
>>>>>>>
>>>>>>> Thanks for explaination. I tried to flip order for lhs/rhs in
>>>>>>> gimplify_modify_expr & co. Issue here is that for some cases we are
>>>>>>> relying here on lhs for gimplifying rhs (is_gimple_reg_rhs_or_call vs
>>>>>>> is_gimple_mem_rhs_or_call) and this doesn't work for cases in C++
>>>>>>> like:
>>>>>>>
>>>>>>> typedef const unsigned char _Jv_Utf8Const;
>>>>>>> typedef __SIZE_TYPE__ uaddr;
>>>>>>>
>>>>>>> void maybe_adjust_signature (_Jv_Utf8Const *&s, uaddr &special)
>>>>>>> {
>>>>>>> union {
>>>>>>> _Jv_Utf8Const *signature;
>>>>>>> uaddr signature_bits;
>>>>>>> };
>>>>>>> signature = s;
>>>>>>> special = signature_bits & 1;
>>>>>>> signature_bits -= special;
>>>>>>> s = signature;
>>>>>>> }
>>>>>>>
>>>>>>> So I modified gimplify_self_mod_expr for post-inc/dec so that we use
>>>>>>> following sequence
>>>>>>> and add it to pre_p for it:
>>>>>>>
>>>>>>> tmp = lhs;
>>>>>>> lvalue = tmp (+/-) rhs
>>>>>>> *expr_p = tmp;
>>>>>>
>>>>>> As I explained this is the wrong place to fix the PR. The issue is not
>>>>>> about self-modifying expressions but about evaluating call argument
>>>>>> side-effects before side-effects of the lhs.
>>>>>
>>>>> I am testing the attached instead.
>>>>
>>>> Doesn't work. Btw, Kai, your patch surely breaks things if you put
>>>> the lvalue update into the pre queue.
>>>>
>>>> Consider a simple
>>>>
>>>> a[i++] = i;
>>>>
>>>> you gimplify that to
>>>>
>>>> i.0 = i;
>>>> D.1709 = i.0;
>>>> i = D.1709 + 1;
>>>> a[D.1709] = i;
>>>>
>>>> which is wrong.
>>>>
>>>> Seems we are lacking some basic pre-/post-modify testcases ...
>>>>
>>>> Richard.
>>>
>>> Why, this should be wrong? In fact C specification just says that the
>>> post-inc has to happen at least before next sequence-point. It
>>> doesn't say that the increment has to happen after evaluation of rhs.
>>>
>>> The produced gimple for the following C-code
>>>
>>> int arr[128];
>>>
>>> void foo (int i)
>>> {
>>> arr[i++] = i;
>>> }
>>>
>>> is:
>>>
>>> foo (int i)
>>> {
>>> int D.1364;
>>>
>>> D.1364 = i;
>>> i = D.1364 + 1;
>>> arr[D.1364] = i;
>>> }
>>>
>>> which looks to me from description of the C-specification correct.
>>
>> Hm, indeed. I'll test the following shorter patch and add the struct-return
>> volatile testcase.
>
> Works apart from
>
> Running target unix/
> FAIL: ext/pb_ds/regression/trie_map_rand.cc execution test
> FAIL: ext/pb_ds/regression/trie_set_rand.cc execution test
>
> Maybe invalid testcases. Who knows ... same fails happen with your patch.
>
> Richard.
>
>> Richard.
Hmm, I see in libstdc++'s file include/bits/boost_concept_check.h some
use of '*__i++ = *__i;' and '*__i-- = *__i;', which seems to cause
part of this failure.
This might lead here to this failure. I am not sure, if such
constructs having fixed behavior for C++, but it looks to me like
undefined behavior.
A C-testcase for the issue would be:
int *foo (int *p)
{
*p++ = *p;
return p;
}
which produces with patch now:
foo (int * p)
{
int * D.1363;
int D.1364;
int * D.1365;
D.1363 = p;
p = D.1363 + 4;
D.1364 = *p;
*D.1363 = D.1364;
D.1365 = p;
return D.1365;
}
but in old variant we were producing:
foo (int * p)
{
int D.1363;
int * D.1364;
D.1363 = *p;
*p = D.1363;
p = p + 4;
D.1364 = p;
return D.1364;
}
So, maybe the real solution for this issue might be to swap for
assignment gimplification the order for lhs/rhs gimplification
instead.
Regards,
Kai
