Re: FDO usability: pid handling

2011-04-21 Thread Xinliang David Li
Revised as suggested and committed. Thanks, David On Thu, Apr 21, 2011 at 1:36 PM, Jan Hubicka wrote: >> @@ -730,6 +726,8 @@ void cgraph_clone_inlined_nodes (struct >>  void compute_inline_parameters (struct cgraph_node *); >>  cgraph_inline_failed_t cgraph_edge_inlinable_p (struct cgraph_edge

Re: FDO usability: pid handling

2011-04-21 Thread Jan Hubicka
> On Thu, Apr 21, 2011 at 1:36 PM, Jan Hubicka wrote: > >> @@ -730,6 +726,8 @@ void cgraph_clone_inlined_nodes (struct > >>  void compute_inline_parameters (struct cgraph_node *); > >>  cgraph_inline_failed_t cgraph_edge_inlinable_p (struct cgraph_edge *); > >> > >> +void cgraph_init_node_map (voi

Re: FDO usability: pid handling

2011-04-21 Thread Xinliang David Li
On Thu, Apr 21, 2011 at 1:36 PM, Jan Hubicka wrote: >> @@ -730,6 +726,8 @@ void cgraph_clone_inlined_nodes (struct >>  void compute_inline_parameters (struct cgraph_node *); >>  cgraph_inline_failed_t cgraph_edge_inlinable_p (struct cgraph_edge *); >> >> +void cgraph_init_node_map (void); >> +void

Re: FDO usability: pid handling

2011-04-21 Thread Jan Hubicka
> @@ -730,6 +726,8 @@ void cgraph_clone_inlined_nodes (struct > void compute_inline_parameters (struct cgraph_node *); > cgraph_inline_failed_t cgraph_edge_inlinable_p (struct cgraph_edge *); > > +void cgraph_init_node_map (void); > +void cgraph_del_node_map (void); Given that you don't even

Re: FDO usability: pid handling

2011-04-20 Thread Xinliang David Li
Please review the latest patch. SPEC2k FDO testing pass. Thanks, David On Wed, Apr 20, 2011 at 11:22 AM, Xinliang David Li wrote: > Here is the revised patch. Basic FDO testing went fine. I still saw > the ipa-inline assertion in SPEC FDO. Will run it when the regression > is fixed. > > Thanks,

Re: FDO usability: pid handling

2011-04-20 Thread Xinliang David Li
Discard this version of the patch. I have not updated source properly and the build/test was invalid. David On Wed, Apr 20, 2011 at 11:22 AM, Xinliang David Li wrote: > Here is the revised patch. Basic FDO testing went fine. I still saw > the ipa-inline assertion in SPEC FDO. Will run it when th

Re: FDO usability: pid handling

2011-04-20 Thread Xinliang David Li
Here is the revised patch. Basic FDO testing went fine. I still saw the ipa-inline assertion in SPEC FDO. Will run it when the regression is fixed. Thanks, David On Tue, Apr 19, 2011 at 5:33 PM, Jan Hubicka wrote: >> On Tue, Apr 19, 2011 at 4:49 PM, Jan Hubicka wrote: >> >> Actually, among all

Re: FDO usability: pid handling

2011-04-19 Thread Jan Hubicka
> On Tue, Apr 19, 2011 at 4:49 PM, Jan Hubicka wrote: > >> Actually, among all the choices, funcdef_no is probably the most dense > >> one -- it is for function decl with definition only.  In LIPO, the > > > > Yes, funddef_no is densiest, but we don't really need great density here > > (in many ot

Re: FDO usability: pid handling

2011-04-19 Thread Xinliang David Li
On Tue, Apr 19, 2011 at 4:49 PM, Jan Hubicka wrote: >> Actually, among all the choices, funcdef_no is probably the most dense >> one -- it is for function decl with definition only.  In LIPO, the > > Yes, funddef_no is densiest, but we don't really need great density here > (in many other places w

Re: FDO usability: pid handling

2011-04-19 Thread Jan Hubicka
> Actually, among all the choices, funcdef_no is probably the most dense > one -- it is for function decl with definition only. In LIPO, the Yes, funddef_no is densiest, but we don't really need great density here (in many other places we index arrays by cgraph_uid - it is intended for that purpo

Re: FDO usability: pid handling

2011-04-19 Thread Xinliang David Li
On Tue, Apr 19, 2011 at 4:30 PM, Jan Hubicka wrote: >> On Tue, Apr 19, 2011 at 3:57 PM, Jan Hubicka wrote: >> >> So between hashtab and VEC, which one do you prefer? Either one is fine >> >> with me. >> > >> > I would go with VEC.  While the array will have holes, there are not many >> > since

Re: FDO usability: pid handling

2011-04-19 Thread Jan Hubicka
> On Tue, Apr 19, 2011 at 3:57 PM, Jan Hubicka wrote: > >> So between hashtab and VEC, which one do you prefer? Either one is fine > >> with me. > > > > I would go with VEC.  While the array will have holes, there are not many > > since > > the ids are originally assigned sequentially. > > > > A

Re: FDO usability: pid handling

2011-04-19 Thread Xinliang David Li
On Tue, Apr 19, 2011 at 3:57 PM, Jan Hubicka wrote: >> So between hashtab and VEC, which one do you prefer? Either one is fine with >> me. > > I would go with VEC.  While the array will have holes, there are not many > since > the ids are originally assigned sequentially. > > Actually given that

Re: FDO usability: pid handling

2011-04-19 Thread Jan Hubicka
> So between hashtab and VEC, which one do you prefer? Either one is fine with > me. I would go with VEC. While the array will have holes, there are not many since the ids are originally assigned sequentially. Actually given that we do IPA pass now, I think you can just remove cgraph->pid field

Re: FDO usability: pid handling

2011-04-19 Thread Xinliang David Li
So between hashtab and VEC, which one do you prefer? Either one is fine with me. Thanks, David On Tue, Apr 19, 2011 at 3:39 PM, Jan Hubicka wrote: >> >> Why is VEC any better in terms of density ? Are you suggesting using a >> hash table? > It is not any better, but we usually use VEC for varia

Re: FDO usability: pid handling

2011-04-19 Thread Jan Hubicka
> > Why is VEC any better in terms of density ? Are you suggesting using a > hash table? It is not any better, but we usually use VEC for variably sized arrays like this one. Not that I would be big fan of its API, but at least it fives bounds checking that would catch bugs like one you are fixin

Re: FDO usability: pid handling

2011-04-19 Thread Xinliang David Li
On Tue, Apr 19, 2011 at 2:38 PM, Jan Hubicka wrote: >> Hi, Insane value profile data may contain indirect call targets with >> wrong (corrupted) pids.  r172276 solves the problem when the pid >> refers to a bogus target that is still 'alive'. This patch addresses >> the issue when the bogus target

Re: FDO usability: pid handling

2011-04-19 Thread Jan Hubicka
> Hi, Insane value profile data may contain indirect call targets with > wrong (corrupted) pids. r172276 solves the problem when the pid > refers to a bogus target that is still 'alive'. This patch addresses > the issue when the bogus target is already eliminated or pid is too > large. > > OK aft

FDO usability: pid handling

2011-04-19 Thread Xinliang David Li
Hi, Insane value profile data may contain indirect call targets with wrong (corrupted) pids. r172276 solves the problem when the pid refers to a bogus target that is still 'alive'. This patch addresses the issue when the bogus target is already eliminated or pid is too large. OK after testing? (S