[llvm-commits] [llvm-gcc-4.2] r63087 - /llvm-gcc-4.2/trunk/gcc/tree-nested.c

[Duncan Sands]

Hi Dale,

> > this assertion is not really mysterious.
> 
> Well, it had no comments, no pointer to a bug it fixed, and no test  
> case....

maybe the commit log is helpful:

r45934 | baldrick | 2008-01-13 19:42:27 +0100 (Sun, 13 Jan 2008) | 20 lines

Fix [Bug tree-optimization/30927], which results
in suboptimal code for gcc, but maybe wrong code
for llvm-gcc.  The problem occurs when one nested
function, A, calls another nested function B,
when the body of B is output after the body
of A (so at the moment of the call, A only has
the declaration of B).  The bug results in the
call thinking that B takes a static chain,
whether or not B actually does.  Since gcc does
not have the static chain as an actual function
parameter, but as an implicit one always passed
in a register, it is fairly harmless if A sets
the register value and B never uses it.  But in
LLVM the static chain is a normal function
parameter, and the mismatch between the call
and the callee can result in trouble, not to
mention unpleasant warnings from instcombine about
"While resolving call to function 'B' arguments
were dropped!".

It is true that there is no testcase for this assertion, but how do
you test for something that can't happen (the testcase for the bug
being fixed is FrontendC/2008-01-11-ChainConsistency.c)?  There are
more details in the gcc bug report.

> > It is here
> > because I had to add a bunch of code to fix a long
> > standing tree-nested bug in which nested function bodies
> > and callers have different opinions as to whether
> > the function takes a static chain parameter or not.
> > This doesn't matter much for gcc because the parameter
> > is passed out of line, but it is rather horrible for
> > llvm-gcc because it is added as an explicit parameter
> > to the function.  I never got round to pushing this
> > upstream, but I really should.  Anyway, the assertion
> > is checking that a nested subroutine (the root->outer
> > check ensures that this is a nested subroutine) takes
> > an extra parameter for passing variables belonging to
> > the parent (the "static chain") if and only if either
> > it itself accesses a variable belonging to its parent
> > (in which case root->chain_field is not null) or one
> > of its child nested subroutines does (in which case
> > root->chain_decl is not null).
> >
> >> -      gcc_assert (!root->outer ||
> >> -                  DECL_NO_STATIC_CHAIN (root->context) ==
> >> -                  !(root->chain_decl || root->chain_field));
> >
> > If the assertion is failing, it seems to me that something
> > bad is going on.
> 
> You may be right, or this may be another case where ObjC isn't doing  
> what you expect (which you would probably consider abuse).  I  
> concluded it was the latter, but  you seem to understand what this  
> code is doing better than I, maybe you could look at the testcase?

I will.  Sadly I do understand the horror that is tree-nested, at
least I used to.  I had to in order to implement support for nested
functions in llvm-gcc :( :)

> It's misspelled in the checkin message above, should be block- 
> in_structors.C .

Thanks!

Ciao,

Duncan.