This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, Pointer Bounds Checker 14/x] Pointer Bounds Checker passes
- From: Ilya Enkovich <enkovich dot gnu at gmail dot com>
- To: Richard Biener <richard dot guenther at gmail dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Fri, 26 Sep 2014 14:08:33 +0400
- Subject: Re: [PATCH, Pointer Bounds Checker 14/x] Pointer Bounds Checker passes
- Authentication-results: sourceware.org; auth=none
- References: <20140529111140 dot GC30323 at msticlxl57 dot ims dot intel dot com> <CAFiYyc0jCmehLi5pKtb002QH4Ps63hWA4i369s7GVTrXYr_7Fg at mail dot gmail dot com> <CAMbmDYZ98sqT7yk=YK2dpnsL8KxYSyg3uSnY8h=k8O8eBFgLaQ at mail dot gmail dot com> <CAFiYyc3GiNtsB86P2PZkY=aAnvwD2oUWukck0t5FQr_vNsYktw at mail dot gmail dot com> <CAMbmDYaShaZYxcwUUqtiiG_9f0Otkou4G29GL9A_g_jS+7h4tw at mail dot gmail dot com> <CAFiYyc00qxABgg2pORfcdxBj0ddOkxeEFZquzsZ5+9j_W-75gQ at mail dot gmail dot com> <20140606065450 dot GA65215 at msticlxl57 dot ims dot intel dot com>
Ping
2014-06-06 10:54 GMT+04:00 Ilya Enkovich <enkovich.gnu@gmail.com>:
> On 03 Jun 10:59, Richard Biener wrote:
>> On Mon, Jun 2, 2014 at 5:13 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>> > 2014-06-02 17:37 GMT+04:00 Richard Biener <richard.guenther@gmail.com>:
>> >> On Mon, Jun 2, 2014 at 2:44 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>> >>> 2014-06-02 15:35 GMT+04:00 Richard Biener <richard.guenther@gmail.com>:
>> >>>> On Fri, May 30, 2014 at 2:25 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>> >>>>> Hi,
>> >>>>>
>> >>>>> This patch adds Pointer Bounds Checker passes. Versioning happens before early local passes. Earply local passes are split into 3 stages to have everything instrumented before any optimization applies.
>> >>>>
>> >>>> That looks artificial to me. If you need to split up early_local_passes
>> >>>> then do that - nesting three IPA pass groups inside it looks odd to me.
>> >>>> Btw - doing this in three "IPA phases" makes things possibly slower
>> >>>> due to cache effects. It might be worth pursuing to move the early
>> >>>> stage completely to the lowering pipeline.
>> >>>
>> >>> Early local passes is some special case because these passes are
>> >>> executed separately for new functions. I did not want to get three
>> >>> special passes instead and therefore made split inside.
>> >>
>> >> Yeah, but all passes are already executed via execute_early_local_passes,
>> >> so it would be only an implementation detail.
>> >>
>> >>> If you prefer split pass itself, I suppose pass_early_local_passes may
>> >>> be replaced with something like pass_build_ssa_passes +
>> >>> pass_chkp_instrumentation_passes + pass_ipa_chkp_produce_thunks +
>> >>> pass_local_optimization_passes. execute_early_local_passes would
>> >>> execute gimple passes lists of pass_build_ssa_passes,
>> >>> pass_chkp_instrumentation_passes and pass_local_optimization_passes.
>> >>>
>> >>> I think we cannot have the first stage moved into lowering passes
>> >>> because it should be executed for newly created functions.
>> >>
>> >> Well, let's defer that then.
>> >>
>> >>>>
>> >>>> Btw, fixup_cfg only needs to run once local_pure_const was run
>> >>>> on a callee, thus it shouldn't be neccessary to run it from the
>> >>>> first group.
>> >>>
>> >>> OK. Will try to remove it from the first group.
>> >>>
>> >>>>
>> >>>> void
>> >>>> pass_manager::execute_early_local_passes ()
>> >>>> {
>> >>>> - execute_pass_list (pass_early_local_passes_1->sub);
>> >>>> + execute_pass_list (pass_early_local_passes_1->sub->sub);
>> >>>> + execute_pass_list (pass_early_local_passes_1->sub->next->sub);
>> >>>> + execute_pass_list (pass_early_local_passes_1->sub->next->next->next->sub);
>> >>>> }
>> >>>>
>> >>>> is gross - it should be enough to execute the early local pass list
>> >>>> (obsolete comment with the suggestion above).
>> >>>
>> >>> This function should call only gimple passes for cfun. Therefore we
>> >>> cannot call IPA passes here and has to execute each gimple passes list
>> >>> separately.
>> >>
>> >> Ok, given a different split this would then become somewhat more sane
>> >> anyway.
>> >
>> > Sorry, didn't catch it. Should I try a different split or defer it? :)
>>
>> Please try a different split. Defer moving the first part to the
>> lowering stage.
>>
>> Richard.
>>
>
> Here is a new version with new split with pass_early_local_passes replaced with new three passes. Left execute_early_local_passes unrenamed. Had to fix test gcc.dg/pr37858.c which uses -fdump-ipa-early_local_cleanups option.
>
> I could not get rid of additional pass_fixup_cfg. Its removal caused wrong CFG (call to nonreturn function in a middle of BB) which confused checker logic. I moved this pass into checker passes list.
>
> Bootstrapped and tested on linux-x86_64.
>
> Thanks,
> Ilya
> --
> gcc/
>
> 2014-06-05 Ilya Enkovich <ilya.enkovich@intel.com>
>
> * tree-chkp.c: New.
> * tree-chkp.h: New.
> * rtl-chkp.c: New.
> * rtl-chkp.h: New.
> * Makefile.in (OBJS): Add tree-chkp.o, rtl-chkp.o.
> (GTFILES): Add tree-chkp.c.
> * c-family/c.opt (fchkp-check-incomplete-type): New.
> (fchkp-zero-input-bounds-for-main): New.
> (fchkp-first-field-has-own-bounds): New.
> (fchkp-narrow-bounds): New.
> (fchkp-narrow-to-innermost-array): New.
> (fchkp-optimize): New.
> (fchkp-use-fast-string-functions): New.
> (fchkp-use-nochk-string-functions): New.
> (fchkp-use-static-bounds): New.
> (fchkp-use-static-const-bounds): New.
> (fchkp-treat-zero-dynamic-size-as-infinite): New.
> (fchkp-check-read): New.
> (fchkp-check-write): New.
> (fchkp-store-bounds): New.
> (fchkp-instrument-calls): New.
> (fchkp-instrument-marked-only): New.
> * cppbuiltin.c (define_builtin_macros_for_compilation_flags): Add
> __CHKP__ macro when Pointer Bounds Checker is on.
> * passes.def (pass_ipa_chkp_versioning): New.
> (pass_early_local_passes): Removed.
> (pass_build_ssa_passes): New.
> (pass_fixup_cfg): Moved to pass_chkp_instrumentation_passes.
> (pass_chkp_instrumentation_passes): New.
> (pass_ipa_chkp_produce_thunks): New.
> (pass_local_optimization_passes): New.
> (pass_chkp_opt): New.
> * toplev.c: include tree-chkp.h.
> (compile_file): Add chkp_finish_file call.
> * tree-pass.h (make_pass_ipa_chkp_versioning): New.
> (make_pass_ipa_chkp_produce_thunks): New.
> (make_pass_chkp): New.
> (make_pass_chkp_opt): New.
> (make_pass_early_local_passes): Removed.
> (make_pass_build_ssa_passes): New.
> (make_pass_chkp_instrumentation_passes): New.
> (make_pass_local_optimization_passes): New.
> * tree.h (called_as_built_in): New.
> * builtins.c (called_as_built_in): Not static anymore.
> * passes.c (pass_manager::execute_early_local_passes): Execute
> early passes in three steps.
> (execute_all_early_local_passes): Removed.
> (pass_data_early_local_passes): Removed.
> (pass_early_local_passes): Removed.
> (execute_build_ssa_passes): New.
> (pass_data_build_ssa_passes): New.
> (pass_build_ssa_passes): New.
> (pass_data_chkp_instrumentation_passes): New.
> (pass_chkp_instrumentation_passes): New.
> (pass_data_local_optimization_passes): New.
> (pass_local_optimization_passes): New.
> (-make_pass_early_local_passes): Removed.
> (make_pass_build_ssa_passes): New.
> (make_pass_chkp_instrumentation_passes): New.
> (make_pass_local_optimization_passes): New.
>
> gcc/testsuite
>
> 2014-06-05 Ilya Enkovich <ilya.enkovich@intel.com>
>
> * gcc.dg/pr37858.c: Replace early_local_cleanups pass name
> with build_ssa_passes.