| Message ID | 20100705235036.GC12132@kam.mff.cuni.cz |
|---|---|
| State | New |
| Headers | show |
On Mon, Jul 5, 2010 at 23:50, Jan Hubicka <hubicka@ucw.cz> wrote: > fndecl = TREE_VALUE (v); > + node = cgraph_get_node (fndecl); > + /* When cgraph node is missing and function is public, we can not s/can not/cannot/ Shouldn't this check be earlier instead of when we are trying to fold? But other folders behave this way, so this is OK too. Diego.
> On Mon, Jul 5, 2010 at 23:50, Jan Hubicka <hubicka@ucw.cz> wrote: > > > fndecl = TREE_VALUE (v); > > + node = cgraph_get_node (fndecl); > > + /* When cgraph node is missing and function is public, we can not > > s/can not/cannot/ > > Shouldn't this check be earlier instead of when we are trying to fold? > But other folders behave this way, so this is OK too. Well, this is the first time we actually see the function, so I don't think we can check anything earlier. Honza
On Tue, Jul 6, 2010 at 00:16, Jan Hubicka <hubicka@ucw.cz> wrote: > Well, this is the first time we actually see the function, so I don't think we can > check anything earlier. Ah, yes, never mind. I had the wrong memory about this code. Diego.
On Tue, 6 Jul 2010, Jan Hubicka wrote: > Hi, > this patch solves problem with building Mozilla javascript shell with > -fwhole-program and WHOPR. The problem here is that cloning causes > devirtualization. This devirtualization is recognized during virtual clone > materialization (i.e. late) because Martin did not merge his IPA > devritualization code yet. > > Because WHOPR brings the function static and house it in different file > than the devirtualized call, we get undefined symbol on the partition. > > This problem is not limited to WHOPR only. Same problem can appear in > normal compilation when call to COMDAT function is devirtualized after > COMDAT was declared dead. > > This patch adds neccesary checking and prevents devirtualization when we > can't anymore. Hm, I don't understand. When there is a virtual call we should still have the functions address taken and thus cannot eliminate it. So why can this be a problem? At least I don't see how it can be for non-WHOPR. COMDAT cannot be declared dead when there is still a virtual call to it. What am I missing? Richard. > Bootstrapped/regtested x86_64-linux, OK? > > Honza > > * gimple-fold.c (gimple_fold_obj_type_ref_known_binfo): Check that > function is still available to fold into. > Index: gimple-fold.c > =================================================================== > --- gimple-fold.c (revision 161847) > +++ gimple-fold.c (working copy) > @@ -1351,6 +1351,7 @@ gimple_fold_obj_type_ref_known_binfo (HO > { > HOST_WIDE_INT i; > tree v, fndecl; > + struct cgraph_node *node; > > v = BINFO_VIRTUALS (known_binfo); > i = 0; > @@ -1362,6 +1363,14 @@ gimple_fold_obj_type_ref_known_binfo (HO > } > > fndecl = TREE_VALUE (v); > + node = cgraph_get_node (fndecl); > + /* When cgraph node is missing and function is public, we can not > + devirtualize. This can happen in WHOPR when the actual method > + ends up in other partition, because we found devirtualization > + possibility too late. */ > + if ((!node || !node->analyzed) > + && (!TREE_PUBLIC (fndecl) || DECL_COMDAT (fndecl))) > + return NULL; > return build_fold_addr_expr (fndecl); > } > > >
> On Tue, 6 Jul 2010, Jan Hubicka wrote: > > > Hi, > > this patch solves problem with building Mozilla javascript shell with > > -fwhole-program and WHOPR. The problem here is that cloning causes > > devirtualization. This devirtualization is recognized during virtual clone > > materialization (i.e. late) because Martin did not merge his IPA > > devritualization code yet. > > > > Because WHOPR brings the function static and house it in different file > > than the devirtualized call, we get undefined symbol on the partition. > > > > This problem is not limited to WHOPR only. Same problem can appear in > > normal compilation when call to COMDAT function is devirtualized after > > COMDAT was declared dead. > > > > This patch adds neccesary checking and prevents devirtualization when we > > can't anymore. > > Hm, I don't understand. When there is a virtual call we should still > have the functions address taken and thus cannot eliminate it. So > why can this be a problem? At least I don't see how it can be for > non-WHOPR. COMDAT cannot be declared dead when there is still a > virtual call to it. > > What am I missing? Hmm, in non-WHOPR one would probably need keyed vtable containing pointer to non-keyed COMDAT functions from ancestor. This way we eliminate vtable, but keep vinfos. But I am not sure it happens in reality. With WHOPR it is easy - since we don't see any connection in between function and method before we devirtualize, we end up putting method into different partition. In any case we want devirutalization happen early, so this is not much of problem. Only thing I worry about is that I believe we should handle this already - it is the case when ipa-cp creates clone that allows devirutalizing. This is same to what inliner does. Martin? Honza
Index: gimple-fold.c =================================================================== --- gimple-fold.c (revision 161847) +++ gimple-fold.c (working copy) @@ -1351,6 +1351,7 @@ gimple_fold_obj_type_ref_known_binfo (HO { HOST_WIDE_INT i; tree v, fndecl; + struct cgraph_node *node; v = BINFO_VIRTUALS (known_binfo); i = 0; @@ -1362,6 +1363,14 @@ gimple_fold_obj_type_ref_known_binfo (HO } fndecl = TREE_VALUE (v); + node = cgraph_get_node (fndecl); + /* When cgraph node is missing and function is public, we can not + devirtualize. This can happen in WHOPR when the actual method + ends up in other partition, because we found devirtualization + possibility too late. */ + if ((!node || !node->analyzed) + && (!TREE_PUBLIC (fndecl) || DECL_COMDAT (fndecl))) + return NULL; return build_fold_addr_expr (fndecl); }