On 19 July 2016 at 11:04, Richard Biener <richard.guent...@gmail.com> wrote: > On July 18, 2016 11:05:58 PM GMT+02:00, David Malcolm <dmalc...@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.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. >
Yes, the token '.1' is treated as 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. > As special characters are not valid in C, I am using 'a..1' as syntax for ssa name. For parsing I am using + lhs.value = make_ssa_name_fn (cfun, + lookup_name (c_parser_peek_token (parser)->value), + NULL); Now for passing ssa name into __PHI, how can we lookup for particular ssa name. Or is there other better method to do it? > Richard. > >>Dave > >