This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Ping: Re: [patch middle-end]: Fix PR/48814 - [4.4/4.5/4.6/4.7 Regression] Incorrect scalar increment result
2012/2/10 Richard Guenther <richard.guenther@gmail.com>:
> On Fri, Feb 10, 2012 at 9:36 AM, Kai Tietz <ktietz70@googlemail.com> wrote:
>> 2012/2/9 Richard Guenther <richard.guenther@gmail.com>:
>>> 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.
>
> Well, that would certainly not be suitable for stage4 (though I have a working
> patch for that as well). ?The post-modify gimplification change looks better
> as it also fixes the volatile aggregate-return case which would not be fixed
> by re-ordering of the gimplification.
>
> libstdc++ folks - can you investigate the testsuite failure?
>
> The patch in question is at
> http://gcc.gnu.org/ml/gcc-patches/2012-02/msg00435/fix-pr48814
>
> Note that the _Mutable_ForwardIteratorConcept isn't the problem I think,
> it's not executed code.
>
> Thanks,
> Richard.
Hmm, we might need here lhs/rhs flip plus the post-modify. At least
we would avoid by this code differences for common cases that lhs has
post-inc/dec to current behavior?
But you might be right that this patch is not suitable for stage 4.
Regards,
Kai