Ping: Re: [patch middle-end]: Fix PR/48814 - [4.4/4.5/4.6/4.7 Regression] Incorrect scalar increment result

Kai Tietz ktietz70@googlemail.com
Fri Feb 10 15:20:00 GMT 2012


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



More information about the Gcc-patches mailing list