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: [PATCH] alternative hirate for builtin_expert


I looked at this problem. Bug updated
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58619

This is a bug when updating block during tree-inline. Basically, it is
legal for *n to be NULL. E.g. When gimple_block(id->gimple_call) is
NULL, remap_blocks_to_null will be called to set *n to NULL.

The problem is that we should check this before calling
COMBINE_LOCATION_DATA, which assumes block is not NULL.

The following patch can fix the problem:

Index: gcc/tree-inline.c
===================================================================
--- gcc/tree-inline.c (revision 203208)
+++ gcc/tree-inline.c (working copy)
@@ -2090,7 +2090,10 @@ copy_phis_for_bb (basic_block bb, copy_body_data *
   n = (tree *) pointer_map_contains (id->decl_map,
  LOCATION_BLOCK (locus));
   gcc_assert (n);
-  locus = COMBINE_LOCATION_DATA (line_table, locus, *n);
+  if (*n)
+    locus = COMBINE_LOCATION_DATA (line_table, locus, *n);
+  else
+    locus = LOCATION_LOCUS (locus);
  }
       else
  locus = LOCATION_LOCUS (locus);

On Fri, Oct 4, 2013 at 11:05 AM, Rong Xu <xur@google.com> wrote:
> My change on the probability of builtin_expect does have an impact on
> the partial inline (more outlined functions will get inline back to
> the original function).
> I think this triggers an existing issue.
> Dehao will explain this in his coming email.
>
> -Rong
>
> On Fri, Oct 4, 2013 at 6:05 AM, Ramana Radhakrishnan <ramrad01@arm.com> wrote:
>> On 10/02/13 23:49, Rong Xu wrote:
>>>
>>> Here is the new patch. Honaz: Could you take a look?
>>>
>>> Thanks,
>>>
>>> -Rong
>>>
>>> On Wed, Oct 2, 2013 at 2:31 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>>>>>
>>>>> Thanks for the suggestion. This is much cleaner than to use binary
>>>>> parameter.
>>>>>
>>>>> Just want to make sure I understand it correctly about the orginal
>>>>> hitrate:
>>>>> you want to retire the hitrate in PRED_BUILTIN_EXPECT and always use
>>>>> the one specified in the biniltin-expect-probability parameter.
>>>>
>>>>
>>>> Yes.
>>>>>
>>>>>
>>>>> Should I use 90% as the default? It's hard to fit current value 0.9996
>>>>> in percent form.
>>>>
>>>>
>>>> Yes, 90% seems fine.  The original value was set quite arbitrarily and no
>>>> real
>>>> performance study was made as far as I know except yours. I think users
>>>> that
>>>> are sure they use expect to gueard completely cold edges may just use
>>>> 100%
>>>> instead of 0.9996, so I would not worry much about the precision.
>>>>
>>>> Honza
>>>>>
>>>>>
>>>>> -Rong
>>>>>>
>>>>>>
>>>>>> OK with that change.
>>>>>>
>>>>>> Honza
>>
>>
>>
>> This broke arm-linux-gnueabihf building libstdc++-v3. I haven't dug further
>> yet but still reducing the testcase.
>>
>> See
>>
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58619
>>
>> for details.
>>
>> Can you please deal with this appropriately ?
>>
>> regards
>> Ramana
>>
>>


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