On Tue, 2016-07-19 at 00:52 +0530, Prasad Ghangal wrote:
> On 19 July 2016 at 00:25, Richard Biener <richard.guent...@gmail.com>
> wrote:
> > On July 18, 2016 8:28:15 PM GMT+02:00, Prasad Ghangal <
> > prasad.ghan...@gmail.com> wrote:
> > > On 15 July 2016 at 16:13, Richard Biener <
> > > richard.guent...@gmail.com>
> > > wrote:
> > > > On Sun, Jul 10, 2016 at 6:13 PM, Prasad Ghangal
> > > > <prasad.ghan...@gmail.com> wrote:
> > > > > On 8 July 2016 at 13:13, Richard Biener <
> > > > > richard.guent...@gmail.com>
> > > wrote:
> > > > > > On Thu, Jul 7, 2016 at 9:45 PM, Prasad Ghangal
> > > <prasad.ghan...@gmail.com> wrote:
> > > > > > > On 6 July 2016 at 14:24, Richard Biener
> > > <richard.guent...@gmail.com> wrote:
> > > > > > > > On Wed, Jul 6, 2016 at 9:51 AM, Prasad Ghangal
> > > <prasad.ghan...@gmail.com> wrote:
> > > > > > > > > On 30 June 2016 at 17:10, Richard Biener
> > > <richard.guent...@gmail.com> wrote:
> > > > > > > > > > On Wed, Jun 29, 2016 at 9:13 PM, Prasad Ghangal
> > > > > > > > > > <prasad.ghan...@gmail.com> wrote:
> > > > > > > > > > > On 29 June 2016 at 22:15, Richard Biener
> > > <richard.guent...@gmail.com> wrote:
> > > > > > > > > > > > On June 29, 2016 6:20:29 PM GMT+02:00,
> > > > > > > > > > > > Prathamesh Kulkarni
> > > <prathamesh.kulka...@linaro.org> wrote:
> > > > > > > > > > > > > On 18 June 2016 at 12:02, Prasad Ghangal
> > > <prasad.ghan...@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.
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?
Dave
