This is the mail archive of the 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: CFG transparent inlining

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 ();

No aborts.

> *************** 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 
glossed over.

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.


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