This is the mail archive of the
mailing list for the GCC project.
Re: CFG transparent inlining
- From: Richard Henderson <rth at redhat dot com>
- To: Jan Hubicka <jh at suse dot cz>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Mon, 9 May 2005 17:18:11 -0700
- Subject: Re: CFG transparent inlining
- References: <20050429134201.GM29212@kam.mff.cuni.cz>
On Fri, Apr 29, 2005 at 03:42:01PM +0200, Jan Hubicka wrote:
> ! /* After inlining function marked NOTHROW into MUST_NOT_THROW region
> ! with throwing instructions we may end up with missing EH landing
> ! pad for the exceptions. It seems safe to ignore them (by setting
> ! NOTHOROW flag on the function user promised this scenario to not
> ! happen). See filter2.C testcase. */
> ! if (XEXP (i, 0))
> ! make_label_edge (edge_cache, src, XEXP (i, 0),
> ! EDGE_ABNORMAL | EDGE_EH | is_call);
You're going to have to give some justification for this. I can't think
of why you would have to be taking care of this here, rather than up at
the tree level.
> + static struct eh_region *
> + duplicate_eh_region_1 (struct eh_region *o)
Why isn't this function just a structure assignment? I.e.
n = allocate (size)
*n = *o;
> + default:
> + abort ();
> *************** reachable_next_level (struct eh_region *
> ! Be conservative before inlining since new subregions may be
> ! introduced. fixup_cfg pass takes care of removing unnecesary
> ! EH edges later on. */
> ! if ((info && info->saw_any_handlers) || !cfun->after_inlining)
I don't understand. With reachable_next_level, you search up, not down.
Inlining adds *sub*regions, meaning things that are below what currently
exists. At no point are we adding regions in the middle of the tree;
only at the leaves.
If this is supposed to be solving a real problem, then I think you're
solving it at the wrong place.
> + duplicate_stmt_eh_region_mapping (struct function *ifun,
> + struct function *cfun,
Please don't use parameters of the same name as a well-known global.
> ! /* Exception region the inlined call like in. */
> + get_eh_last_region_number (DECL_STRUCT_FUNCTION (id->caller))
> + - get_eh_last_region_number (id->callee_cfun)
> + + TREE_INT_CST_LOW (TREE_OPERAND (*tp, 0)));
It would be nice if this operation were abstracted.
> + duplicate_eh_regions (cfun_to_copy, (duplicate_eh_regions_map)remap_decl,
Why this cast? Casting functions is Bad.
> ! copy_res_decl_for_inlining (tree res, tree fn, tree caller,
> ! void *dm ATTRIBUTE_UNUSED,
> ! int *ndp ATTRIBUTE_UNUSED,
> ! tree return_slot_addr ATTRIBUTE_UNUSED)
Unused parameters in a new function?
> ! /* All passes needed to lower the function into shape optimizers can operate
> ! on. We need these to be separate from local optimization because C++ needs
> ! to go into lowered form earlier to perfrom template instantiation. */
Hmm? Care to explain?
That's about all I could see on this pass through. But much was
I'd like to not see this again as a 3500 line patch. I know there are
parts of it that are independent bug fixes; lets get those in first.
I know there are parts that are just infrastructure, and could go in
without being used.