This is the mail archive of the
gcc@gcc.gnu.org
mailing list for the GCC project.
Re: [gimplefe] hacking pass manager
On July 18, 2016 11:05:58 PM GMT+02:00, David Malcolm <dmalcolm@redhat.com> wrote:
>On Tue, 2016-07-19 at 00:52 +0530, Prasad Ghangal wrote:
>> On 19 July 2016 at 00:25, Richard Biener <richard.guenther@gmail.com>
>> wrote:
>> > On July 18, 2016 8:28:15 PM GMT+02:00, Prasad Ghangal <
>> > prasad.ghangal@gmail.com> wrote:
>> > > On 15 July 2016 at 16:13, Richard Biener <
>> > > richard.guenther@gmail.com>
>> > > wrote:
>> > > > On Sun, Jul 10, 2016 at 6:13 PM, Prasad Ghangal
>> > > > <prasad.ghangal@gmail.com> wrote:
>> > > > > On 8 July 2016 at 13:13, Richard Biener <
>> > > > > richard.guenther@gmail.com>
>> > > wrote:
>> > > > > > On Thu, Jul 7, 2016 at 9:45 PM, Prasad Ghangal
>> > > <prasad.ghangal@gmail.com> wrote:
>> > > > > > > On 6 July 2016 at 14:24, Richard Biener
>> > > <richard.guenther@gmail.com> wrote:
>> > > > > > > > On Wed, Jul 6, 2016 at 9:51 AM, Prasad Ghangal
>> > > <prasad.ghangal@gmail.com> wrote:
>> > > > > > > > > On 30 June 2016 at 17:10, Richard Biener
>> > > <richard.guenther@gmail.com> wrote:
>> > > > > > > > > > On Wed, Jun 29, 2016 at 9:13 PM, Prasad Ghangal
>> > > > > > > > > > <prasad.ghangal@gmail.com> wrote:
>> > > > > > > > > > > On 29 June 2016 at 22:15, Richard Biener
>> > > <richard.guenther@gmail.com> wrote:
>> > > > > > > > > > > > On June 29, 2016 6:20:29 PM GMT+02:00,
>> > > > > > > > > > > > Prathamesh Kulkarni
>> > > <prathamesh.kulkarni@linaro.org> wrote:
>> > > > > > > > > > > > > On 18 June 2016 at 12:02, Prasad Ghangal
>> > > <prasad.ghangal@gmail.com>
>> > > > > > > > > > > > > wrote:
>> > > > > > > > > > > > > > Hi,
>> > > > > > > > > > > > > >
>> > > > > > > > > > > > > > I tried hacking pass manager to execute
>> > > > > > > > > > > > > > only given passes.
>> > > For this I
>> > > > > > > > > > > > > > am adding new member as opt_pass
>> > > > > > > > > > > > > > *custom_pass_list to the
>> > > function
>> > > > > > > > > > > > > > structure to store passes need to execute
>> > > > > > > > > > > > > > and providing the
>> > > > > > > > > > > > > > custom_pass_list to execute_pass_list()
>> > > > > > > > > > > > > > function instead of
>> > > all
>> > > > > > > > > > > > > passes
>> > > > > > > > > > > > > >
>> > > > > > > > > > > > > > for test case like-
>> > > > > > > > > > > > > >
>> > > > > > > > > > > > > > int a;
>> > > > > > > > > > > > > > void __GIMPLE (execute ("tree-ccp1", "tree
>> > > > > > > > > > > > > > -fre1")) foo()
>> > > > > > > > > > > > > > {
>> > > > > > > > > > > > > > bb_1:
>> > > > > > > > > > > > > > a = 1 + a;
>> > > > > > > > > > > > > > }
>> > > > > > > > > > > > > >
>> > > > > > > > > > > > > > it will execute only given passes i.e. ccp1
>> > > > > > > > > > > > > > and fre1 pass
>> > > on the
>> > > > > > > > > > > > > function
>> > > > > > > > > > > > > >
>> > > > > > > > > > > > > > and for test case like -
>> > > > > > > > > > > > > >
>> > > > > > > > > > > > > > int a;
>> > > > > > > > > > > > > > void __GIMPLE (startwith ("tree-ccp1"))
>> > > > > > > > > > > > > > foo()
>> > > > > > > > > > > > > > {
>> > > > > > > > > > > > > > bb_1:
>> > > > > > > > > > > > > > a = 1 + a;
>> > > > > > > > > > > > > > }
>> > > > > > > > > > > > > >
>> > > > > > > > > > > > > > it will act as a entry point to the
>> > > > > > > > > > > > > > pipeline and will
>> > > execute passes
>> > > > > > > > > > > > > > starting from given pass.
>> > > > > > > > > > > > > Bike-shedding:
>> > > > > > > > > > > > > Would it make sense to have syntax for
>> > > > > > > > > > > > > defining pass ranges
>> > > to execute
>> > > > > > > > > > > > > ?
>> > > > > > > > > > > > > for instance:
>> > > > > > > > > > > > > void __GIMPLE(execute (pass_start :
>> > > > > > > > > > > > > pass_end))
>> > > > > > > > > > > > > which would execute all the passes within
>> > > > > > > > > > > > > range [pass_start,
>> > > pass_end],
>> > > > > > > > > > > > > which would be convenient if the range is
>> > > > > > > > > > > > > large.
>> > > > > > > > > > > >
>> > > > > > > > > > > > But it would rely on a particular pass
>> > > > > > > > > > > > pipeline, f.e.
>> > > pass-start appearing before pass-end.
>> > > > > > > > > > > >
>> > > > > > > > > > > > Currently control doesn't work 100% as it only
>> > > > > > > > > > > > replaces
>> > > all_optimizations but not lowering passes or early opts, nor IPA
>> > > opts.
>> > > > > > > > > > > >
>> > > > > > > > > > >
>> > > > > > > > > > > Each pass needs GIMPLE in some specific form. So
>> > > > > > > > > > > I am letting
>> > > lowering
>> > > > > > > > > > > and early opt passes to execute. I think we have
>> > > > > > > > > > > to execute
>> > > some
>> > > > > > > > > > > passes (like cfg) anyway to represent GIMPLE into
>> > > > > > > > > > > proper form
>> > > > > > > > > >
>> > > > > > > > > > Yes, that's true. Note that early opt passes only
>> > > > > > > > > > optimize but
>> > > we need
>> > > > > > > > > > pass_build_ssa_passes at least (for into-SSA). For
>> > > > > > > > > > proper
>> > > unit-testing
>> > > > > > > > > > of GIMPLE passes we do need to guard off early opts
>> > > > > > > > > > somehow
>> > > > > > > > > > (I guess a simple if (flag_gimple && cfun
>> > > > > > > > > > ->custom_pass_list)
>> > > would do
>> > > > > > > > > > that).
>> > > > > > > > > >
>> > > > > > > > > > Then there is of course the question about IPA
>> > > > > > > > > > passes which I
>> > > think is
>> > > > > > > > > > somewhat harder (one could always disable all IPA
>> > > > > > > > > > passes
>> > > manually
>> > > > > > > > > > via flags of course or finally have a global
>> > > > > > > > > > -fipa/no-ipa like
>> > > most
>> > > > > > > > > > other compilers).
>> > > > > > > > > >
>> > > > > > > > > Can we iterate through all ipa passes and do
>> > > > > > > > > -fdisable-ipa-pass
>> > > or
>> > > > > > > > > -fenable-ipa-pass equivalent for each?
>> > > > > > > >
>> > > > > > > > We could do that, yes. But let's postpone this issue.
>> > > > > > > > I think
>> > > that
>> > > > > > > > startwith is going to be most useful and rather than
>> > > > > > > > constructing
>> > > > > > > > a pass list for it "native" support for it in the pass
>> > > > > > > > manager is
>> > > > > > > > likely to produce better results (add a 'startwith'
>> > > > > > > > member
>> > > alongside
>> > > > > > > > the pass list member and if it is set the pass manager
>> > > > > > > > skips all
>> > > > > > > > passes that do not match 'startwith' and once it
>> > > > > > > > reaches it it
>> > > clears
>> > > > > > > > the field).
>> > > > > > > >
>> > > > > > > > In the future I hope we can get away from a static pass
>> > > > > > > > list and
>> > > more
>> > > > > > > > towards rule-driven pass execution (we have all those
>> > > > > > > > PROP_*
>> > > stuff
>> > > > > > > > already but it isn't really used for example). But
>> > > > > > > > well, that
>> > > would be
>> > > > > > > > a separate GSoC project ;)
>> > > > > > > >
>> > > > > > > > IMHO startwith will provide everything needed for unit
>> > > > > > > > -testing.
>> > > We can
>> > > > > > > > add a flag on whether further passes should be executed
>> > > > > > > > or not
>> > > and
>> > > > > > > > even a pass list like execute ("ccp1", "fre") can be
>> > > > > > > > implemented
>> > > by
>> > > > > > > > startwith ccp1 and then from there executing the rest
>> > > > > > > > of the
>> > > passes in the
>> > > > > > > > list and stopping at the end.
>> > > > > > > >
>> > > > > > > > As said, unit-testing should exercise a single pass if
>> > > > > > > > we can
>> > > control
>> > > > > > > > its input.
>> > > > > > > >
>> > > > > > > In this patch I am skipping execution of passes until
>> > > pass_startwith
>> > > > > > > is found. Unlike previous build, now pass manager
>> > > > > > > executes all
>> > > passes
>> > > > > > > in pipeline starting from pass_startwith instead of just
>> > > > > > > sub
>> > > passes.
>> > > > > >
>> > > > > > That looks good. I wonder if
>> > > > > >
>> > > > > > + if (startwith_p && cfun->startwith)
>> > > > > > + {
>> > > > > > + if (pass->name == cfun->pass_startwith->name
>> > > > > > + || pass->name == "*clean_state")
>> > > > > >
>> > > > > > need better be strcmp ()s though. Also the early
>> > > > > > optimization
>> > > pipeline
>> > > > > > should be executed with startwith support as well.
>> > > > > >
>> > > > >
>> > > > > This patch adds startwith support for early opt passes. But
>> > > > > for
>> > > > > starting from some passes (like asan0, optimized) in
>> > > > > all_passes
>> > > > > pipeline, it falils at verify_curr_properties in
>> > > > > execute_one_pass
>> > > ().
>> > > > > I wonder if we need to update properties after skipping each
>> > > > > pass
>> > > >
>> > > > Yeah, it's not possible to start at arbitrary points with
>> > > > skipping
>> > > passes
>> > > > that provide a required property. I suspect it's good enough
>> > > > that
>> > > we'll
>> > > > ICE if that happens.
>> > > >
>> > > > I see you are working on the dump-file side a bit now. What is
>> > > > still
>> > > > missing after you got support for PHIs is parsing of SSA names.
>> > > > Without this unit-testing will be difficult at best.
>> > > >
>> > > > I think what we need to do is simplify the job of the parser
>> > > > and
>> > > > make the syntax we use to print SSA names a bit different.
>> > > > So rather than printing VAR_VERSION we need to choose a
>> > > > letter that is not part of a valid identifier before VERSION,
>> > > > like a dot '.'. Thus we'd have i.1 instead of i_1 and we'd
>> > > > have
>> > > > .2 instead of _2 for an anonymous SSA name. The advantage
>> > > > for non-anonymous names is that we can properly re-use the
>> > > > C frontends decl handling for declaring and looking up 'i'.
>> > > > The disadvantage is that for anonymous SSA names this isn't
>> > > > so easy which means we could choose to not support those
>> > > > for the moment and require fake decls for them. In fact
>> > > > into-SSA will do the correct thing if we just treat them as
>> > > > decls,
>> > > > thus continue to dump them as _VERSION.
>> > > >
>> > >
>> > > I am little confused here about parsing 'i.1' because lexer drops
>> > > DOT
>> > > token for syntax like NAME.NUMBER . Hence instead of 'i.1' parser
>> > > receives 'i1'
>> >
>> > Are you sure? You should get three tokens, one for 'i', one for the
>> > dot and
>> > One for '1'. You'd lookup the first via the C frontend symbol
>> > table only.
>> >
>>
>> Yes, I was also expecting that. For syntax like 'a.b' (ie name.name)
>> it gives proper 3 tokens but for syntax like 'a.1' it produces only
>> 2.
>>
>> This is what I observed while debugging "int a.1;"
>>
>> (gdb) print *c_parser_peek_nth_token (parser, 1)
>> $3 = {type = CPP_KEYWORD, id_kind = C_ID_NONE, keyword = RID_INT,
>> pragma_kind = PRAGMA_NONE, location = 242114, value =
>> 0x7ffff65c82d0}
>> (gdb) print *c_parser_peek_nth_token (parser, 2)
>> $4 = {type = CPP_NAME, id_kind = C_ID_ID, keyword = RID_MAX,
>> pragma_kind = PRAGMA_NONE, location = 242240, value =
>> 0x7ffff66d0b90}
>> (gdb) print *c_parser_peek_nth_token (parser, 3)
>> $5 = {type = CPP_NUMBER, id_kind = C_ID_NONE, keyword = RID_MAX,
>> pragma_kind = PRAGMA_NONE, location = 242273, value =
>> 0x7ffff66e0030}
>
>What is the number? I'm wondering if it's somehow been lexed as a
>decimal ".1" i.e. if the "." is somehow being treated as part of the
>CPP_NUMBER.
Ah, possible...
>FWIW, I find hacking in calls to "inform" very useful for seeing what
>each token is (assuming that caret printing isn't disabled).
>
> (gdb) call inform ($3->location, "")
> (gdb) call inform ($4->location, "")
> (gdb) call inform ($5
>->location, "")
>
>etc
>
>BTW, does it have to be '.' as the SSA_NAME separator? Could it be a
>different character e.g. '@' or something else that's non-legal in C?
It doesn't have to be '.', but sth visually not too disturbing would be nice. If we don't go with '.' We can as well try to parse the SSA version from i_1, leaving the ambiguity with it being a valid C identifier.
Richard.
>Dave