On Tue, Mar 24, 2015 at 3:06 PM, Jakub Jelinek <[email protected]> wrote:
> On Tue, Mar 24, 2015 at 12:22:27PM +0300, Ilya Enkovich wrote:
>> 2015-03-24 11:33 GMT+03:00 Jakub Jelinek <[email protected]>:
>> > On Thu, Mar 19, 2015 at 11:29:44AM +0300, Ilya Enkovich wrote:
>> >> + /* We might propagate instrumented function pointer into
>> >> + not instrumented function and vice versa. In such a
>> >> + case we need to either fix function declaration or
>> >> + remove bounds from call statement. */
>> >> + if (callee)
>> >> + skip_bounds = chkp_redirect_edge (e);
>> >
>> > I just want to say that I'm not really excited about all this compile time
>> > cost that is added everywhere unconditionally for chkp.
>> > I think much better would be to guard most of it with proper option check
>> > first and only do the more expensive part if the option has been used.
>>
>> Agree, overhead for not instrumented code should be minimized.
>> Unfortunately there is no option check I can use to guard chkp codes
>> due to LTO. Currently it is allowed to pass -fcheck-pointer-bounds for
>> IL generation and don't pass it for final code generation. I suppose I
>> may set this (or some new) flag if see instrumented node when read
>> cgraph and then use it to guard chkp related codes. Would it be
>> acceptable?
>
> The question is what you want to do in the LTO case for the different cases,
> in particular a TU compiled with -fcheck-pointer-bounds and LTO link without
> that, or TU compiled without -fcheck-pointer-bounds and LTO link with it.
> It could be handled as LTO incompatible option, where lto1 would error out
> if you try to mix -fcheck-pointer-bounds with -fno-check-pointer-bounds
> code, or e.g. similar to var-tracking, you could consider adjusting the IL
> upon LTO reading if if some TU has been built with -fcheck-pointer-bounds
> and the LTO link is -fno-check-pointer-bounds. Dunno what will happen
> with -fno-check-pointer-bounds TUs LTO linked with -fcheck-pointer-bounds.
> Or another possibility is to or in -fcheck-pointer-bounds from all TUs.
>
>> Maybe replace attribute usage with a new flag in tree_decl_with_vis
>> structure?
>
> Depends, might be better to stick it into cgraph_node instead, depends on
> whether you are querying it already early in the FEs or just during GIMPLE
> when the cgraph node should be created too.
I also wonder why it is necessary to execute pass_chkp_instrumentation_passes
when mpx is not active.
That is, can we guard that properly in
void
pass_manager::execute_early_local_passes ()
{
execute_pass_list (cfun, pass_build_ssa_passes_1->sub);
execute_pass_list (cfun, pass_chkp_instrumentation_passes_1->sub);
execute_pass_list (cfun, pass_local_optimization_passes_1->sub);
}
(why's that so oddly wrapped?)
class pass_chkp_instrumentation_passes
also has no gate that guards with flag_mpx or so.
That would save a IL walk over all functions (fixup_cfg) and a cgraph
edge rebuild.
Richard.
> Jakub
