This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


2012/2/9 Richard Guenther <richard.guenther@gmail.com>:
> On Thu, Feb 9, 2012 at 11:53 AM, Richard Guenther
> <richard.guenther@gmail.com> wrote:
>> On Thu, Feb 9, 2012 at 11:29 AM, Richard Guenther
>> <richard.guenther@gmail.com> wrote:
>>> On Wed, Feb 8, 2012 at 10:25 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>>>> 2012/1/11 Richard Guenther <richard.guenther@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.

Kai


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]