Bug 48763 - Inliner type ICE
Summary: Inliner type ICE
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: lto (show other bugs)
Version: 4.6.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
: 48761 (view as bug list)
Depends on:
Blocks: mozillametabug
  Show dependency treegraph
 
Reported: 2011-04-25 16:57 UTC by Jan Hubicka
Modified: 2013-03-08 14:05 UTC (History)
0 users

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2011-04-26 10:09:29


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Hubicka 2011-04-25 16:57:43 UTC
Hi,
Mozilla now gets the following ICE:

jh@evans:~> cat t1.C
extern "C" {
  void StoreSpline(int ax, int ay, int bx, int by, int cx, int cy, int dx, int dy);
}
static int crash_ax;
static int crash_ay;
static int crash_bx;
static int crash_by;
static int crash_cx;
static int crash_cy;
static int crash_dx;
static int crash_dy;

void
StoreSpline(int ax, int ay, int bx, int by, int cx, int cy, int dx, int dy) {
    crash_ax = ax;
    crash_ay = ay;
    crash_bx = bx;
    crash_by = by;
    crash_cx = cx;
    crash_cy = cy;
    crash_dx = dx;
    crash_dy = dy;
}

jh@evans:~> cat t2.c
void StoreSpline(int ax, int ay, int bx, int by, int cx, double cy, int dx, int dy);
int a;
double b;
main(void)
{
  StoreSpline (a,a,a,a,a,b,a,a);
}
jh@evans:~> /abuild/jh/trunk-install/bin/gcc -O3 -flto t2.c t1.C 
In file included from :1:0:
t2.c: In function 'main':
t2.c:4:1: error: conversion of register to a different size
VIEW_CONVERT_EXPR<int>(b.1_3);

cy_11 = VIEW_CONVERT_EXPR<int>(b.1_3);

t2.c:4:1: internal compiler error: verify_gimple failed
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.
lto-wrapper: /abuild/jh/trunk-install/bin/gcc returned 1 exit status
collect2: lto-wrapper returned 1 exit status


Seems the code marking edges uninlinable during type merging doen't quite do the job.
Comment 1 Jan Hubicka 2011-04-25 17:04:55 UTC
OK,
the problem is that we check only return types for compatibility:
244       /* Redirect all incoming edges.  */
245       compatible_p
246         = gimple_types_compatible_p (TREE_TYPE (TREE_TYPE (prevailing_node->decl)),
247                                      TREE_TYPE (TREE_TYPE (node->decl)), GTC_DIAG);

Honza
Comment 2 Richard Biener 2011-04-26 10:08:55 UTC
*** Bug 48761 has been marked as a duplicate of this bug. ***
Comment 3 Richard Biener 2011-04-26 10:09:29 UTC
Well.  That specific place isn't proper now that the decls can be compatible
but the call stmt can use a different type for calling.

Any checking should extend/factor gimple_check_call_matching_types.
Comment 4 Jan Hubicka 2011-04-26 12:56:30 UTC
I would say that PR48761 testcase is not 100% dup of this one.  One tests that cgraph merging check type compatibility of direct call edges, the second tests that ipa-prop does type compatibility check when turing indirect edge to direct.

I guess only way to go is to stomp call expression types to the callgraph edges. Doable, but ugly since it will add need to store types into callgraph sections that are currently simple blocks. Or to make inliner and ipa-prop to handle all cases, even those not type compatible, right?

This is not really my area, so I would hope someone to beat me on this problem :))
Comment 5 Richard Biener 2011-04-26 13:05:32 UTC
(In reply to comment #4)
> I would say that PR48761 testcase is not 100% dup of this one.  One tests that
> cgraph merging check type compatibility of direct call edges, the second tests
> that ipa-prop does type compatibility check when turing indirect edge to
> direct.

The issue is the same, it will manifest in all IPA passes, with and without
LTO now that we happily create direct calls from originally mismatching indirect
calls.

> I guess only way to go is to stomp call expression types to the callgraph
> edges. Doable, but ugly since it will add need to store types into callgraph
> sections that are currently simple blocks. Or to make inliner and ipa-prop to
> handle all cases, even those not type compatible, right?
> 
> This is not really my area, so I would hope someone to beat me on this problem
> :))

We should defer detection of incompatibilities to when we see the call stmt,
thus expand_call_inline.  We shouldn't worry about this disturbing inliner
heuristics too much.  IPA optimizations should do propagation as-if things
match, deal with missing arguments (or missing arg slots) properly by
giving up and do the same checking when applying a transform.

I don't think trying to move more information to WPA stages on the edges
is the way to go.
Comment 6 Jan Hubicka 2011-04-26 13:20:24 UTC
> We should defer detection of incompatibilities to when we see the call stmt,
> thus expand_call_inline.  We shouldn't worry about this disturbing inliner

Undoing the inliner decisions at expand_call_inline is not trivial.

1) We would have to turn inline clone back to offline copy of the
function if one doesn't exist.
2) Doing so would resoult in duplicated offline copies of function when such
copy really exist but WPA made it static and put into a different partition.
3) With the inliner analysis being able to figure out what references and
calls will be optimized out in particular inline clone based on knowledge of
its argument, there is no way to output those references (they might refer
things not exported from different WPA partitions).
So the offline clone would have to be clone with the constants figured out by
the inliner propagated in.

I am not sure if those are all problems, but I guess we can go this way,
but it is remarkably ugly.

Also we need smilar kind of validation for ipa-prop.  Jump functions will
get confused on calls with mismatching types. This would mean that the constant
propagation trick above might fail making things even more messy (ie. propagating
constant into a structure argument or so).

Honza
Comment 7 Richard Biener 2013-03-08 14:05:36 UTC
Fixed.  We substitute zero for the mismatched parameter.  Eh.