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: Inliner for Java


Per Bothner writes:
 > I can't say I've done a comprehensive review, but I say it might
 > as well go in.
 > 
 > You might consider geting rid of some #ifdef INLINER_FOR_JAVA
 > conditionals if they just guard code that isn't used by the
 > C inliner, but does use standard tree nodes.  For example:
 > 
 > + #ifdef INLINER_FOR_JAVA
 > +     case EXIT_BLOCK_EXPR:
 > +       WALK_SUBTREE_TAIL (TREE_OPERAND (*tp, 1));
 > +
 > +     case SAVE_EXPR:
 > +       WALK_SUBTREE_TAIL (TREE_OPERAND (*tp, 0));
 > + #endif /* INLINER_FOR_JAVA */
 > 
 > This doesn't really need the #ifdef.  One can argue that adding
 > the #ifdef keeps the C inliner smaller, but ultimately we want
 > a single inliner.

I considered that, but I prioritized the fact that, after
preprocessing, my changes don't affect the C inliner and therefore
can't possibly break anything.  You might argue that this is overly
conservative.  In retrospect I agree with you.  However, I've just
spent a couple of hours bootstrapping, so...

 > Similarly:
 > 
 > + #else /* INLINER_FOR_JAVA */
 > +   else if (TREE_CODE (*tp) == LABELED_BLOCK_EXPR)
 > ...
 > +   else if (TREE_CODE (*tp) == EXIT_BLOCK_EXPR)
 > ...
 > + #endif /* INLINER_FOR_JAVA */
 > 
 > could be just:
 > 
 > + #endif /* ! INLINER_FOR_JAVA */
 > +   else if (TREE_CODE (*tp) == LABELED_BLOCK_EXPR)
 > ...
 > +   else if (TREE_CODE (*tp) == EXIT_BLOCK_EXPR)
 > ...
 > 
 > When it comes to add_stmt_to_compound, why is there a test
 > for !stmt?  Is there any case where it might be called with
 > a null statement?

There was one case where allowing add_stmt_to_compound to take a null
stmt removed some special case code.  However, I think that it's not
required by the current version of the inliner.  It seems to me that
removing the precondition (stmt != null) is mostly a good thing, but I
can see that this is perhaps a matter of taste.

 > Also, I think the test for 'existing' might be wrong.  Shouldn't
 > it be checking (existing != empty_stmt_node) instead?

I don't think so because in the test cases I tried all compound
statements were constructed by beginning with a NULL_TREE and then
adding further statements.  However, let's say that this isn't always
true so in some cases we might construct a node that is, for example,

   (compound_expr (empty_stmt_node) (assignment))

Will this do any harm?  I cannot think of any cases where it might.

Andrew.


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