Thanks,
Prasad
On 29 July 2016 at 06:56, Prathamesh Kulkarni
<prathamesh.kulka...@linaro.org> wrote:
> On 29 July 2016 at 00:01, Prasad Ghangal <prasad.ghan...@gmail.com> wrote:
>> On 27 July 2016 at 14:22, Richard Biener <richard.guent...@gmail.com> wrote:
>>> On Tue, Jul 26, 2016 at 11:38 PM, Prathamesh Kulkarni
>>> <prathamesh.kulka...@linaro.org> wrote:
>>>> On 27 July 2016 at 00:20, Prasad Ghangal <prasad.ghan...@gmail.com> wrote:
>>>>> On 20 July 2016 at 18:28, Richard Biener <richard.guent...@gmail.com>
>>>>> wrote:
>>>>>> On Wed, Jul 20, 2016 at 1:46 PM, Prathamesh Kulkarni
>>>>>> <prathamesh.kulka...@linaro.org> wrote:
>>>>>>> On 20 July 2016 at 11:34, Richard Biener <richard.guent...@gmail.com>
>>>>>>> wrote:
>>>>>>>> On Tue, Jul 19, 2016 at 10:09 PM, Prasad Ghangal
>>>>>>>> <prasad.ghan...@gmail.com> wrote:
>>>>>>>>> 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?
>>>>>>>>
>>>>>>>> Note that with this you need to preserve SSA versions as used in the
>>>>>>>> source
>>>>>>>> (the 1 in a..1). If you can do that (see below) the way to lookup an
>>>>>>>> SSA name
>>>>>>>> is simply calling 'ssa_name (version)' with version being an integer
>>>>>>>> with the
>>>>>>>> version number.
>>>>>>>>
>>>>>>>> make_ssa_name will simply allocate the next available SSA version so
>>>>>>>> you'll need to add an interface that allows you allocation of a
>>>>>>>> specific
>>>>>>>> SSA version. Like with sth similar to
>>>>>>>>
>>>>>>>> Index: gcc/tree-ssanames.c
>>>>>>>> ===================================================================
>>>>>>>> --- gcc/tree-ssanames.c (revision 238512)
>>>>>>>> +++ gcc/tree-ssanames.c (working copy)
>>>>>>>> @@ -255,7 +255,7 @@
>>>>>>>> used without a preceding definition). */
>>>>>>>>
>>>>>>>> tree
>>>>>>>> -make_ssa_name_fn (struct function *fn, tree var, gimple *stmt)
>>>>>>>> +make_ssa_name_fn (struct function *fn, tree var, gimple *stmt, int
>>>>>>>> version)
>>>>>>>> {
>>>>>>>> tree t;
>>>>>>>> use_operand_p imm;
>>>>>>>> @@ -265,8 +265,17 @@
>>>>>>>> || TREE_CODE (var) == RESULT_DECL
>>>>>>>> || (TYPE_P (var) && is_gimple_reg_type (var)));
>>>>>>>>
>>>>>>>> + if (version != 0)
>>>>>>>> + {
>>>>>>>> + t = make_node (SSA_NAME);
>>>>>>>> + SSA_NAME_VERSION (t) = version;
>>>>>>>> + vec_safe_grow_cleared (SSANAMES (fn), version + 1);
>>>>>>>> + gcc_assert ((*SSANAMES (fn))[version] == NULL);
>>>>>>>> + (*SSANAMES (fn))[version] = t;
>>>>>>>> + ssa_name_nodes_created++;
>>>>>>>> + }
>>>>>>>> /* If our free list has an element, then use it. */
>>>>>>>> - if (!vec_safe_is_empty (FREE_SSANAMES (fn)))
>>>>>>>> + else if (!vec_safe_is_empty (FREE_SSANAMES (fn)))
>>>>>>>> {
>>>>>>>> t = FREE_SSANAMES (fn)->pop ();
>>>>>>>> ssa_name_nodes_reused++;
>>>>>>>> Index: gcc/tree-ssanames.h
>>>>>>>> ===================================================================
>>>>>>>> --- gcc/tree-ssanames.h (revision 237253)
>>>>>>>> +++ gcc/tree-ssanames.h (working copy)
>>>>>>>> @@ -79,7 +79,7 @@
>>>>>>>> extern void init_ssanames (struct function *, int);
>>>>>>>> extern void fini_ssanames (struct function *);
>>>>>>>> extern void ssanames_print_statistics (void);
>>>>>>>> -extern tree make_ssa_name_fn (struct function *, tree, gimple *);
>>>>>>>> +extern tree make_ssa_name_fn (struct function *, tree, gimple *, int);
>>>>>>>> extern void release_ssa_name_fn (struct function *, tree);
>>>>>>>> extern bool get_ptr_info_alignment (struct ptr_info_def *, unsigned
>>>>>>>> int *,
>>>>>>>> unsigned int *);
>>>>>>>>
>>>>>>>> where you can then do sth like
>>>>>>>>
>>>>>>>> ssaname = ssa_name (version);
>>>>>>>> if (!ssaname)
>>>>>>>> ssaname = make_ssa_name_fn (cfun, lookup_name (...), NULL,
>>>>>>>> version);
>>>>>>>>
>>>>>>>> to either lookup an existing or allocate a new SSA name of the desired
>>>>>>>> version.
>>>>>>>>
>>>>>>>> Just for some bike-shedding, I don't like using '..' too much ;)
>>>>>>> Would it be a good idea to modify libcpp's lexer to recognize
>>>>>>> identifier.number as a single token if -fgimple is enabled ?
>>>>>>
>>>>>> I don't think so. As said, we can retain the i_2 syntax as well where
>>>>>> you then
>>>>>> get a single token and to decide whether it is a SSA name do a lookup for
>>>>>> 'i' first and if that succeeds, interpret _2 as a version suffix,
>>>>>> otherwise use it
>>>>>> as full variable name. Re-constructing the 1 from .1 or 17 from .17
>>>>>> CPP_NUMBER
>>>>>> is possible as well of course.
>>>>>>
>>>>>
>>>>> Sorry for the late response. I was little busy due to some family problem.
>>>>>
>>>>> In this patch, I am parsing ssa names as described above. But the
>>>>> problem is non ssa variable also gets renamed
>>>> I assume you're referring to renaming of a to a_5 here ?
>>>> I suppose that's expected since 'a' is gimple reg, ssa pass will
>>>> create ssa name for it.
>>>> Maybe emit "parse error" if local var is not in "ssa syntax", that is,
>>>> it doesn't
>>>> have _version suffixed ? But emitting an error for this case looks
>>>> artificial to me.
>>>
>>> Yes, I think the result is as expected. If we want to constrain the
>>> input to the
>>> GIMPLE parser then we somehow have to tell it what kind of GIMPLE we are
>>> presenting it (SSA or non-SSA gimple), but for the moment what you have
>>> and show with the testcase looks good to me.
>>>
>>> The parsing doesn't handle
>>>
>>> void __GIMPLE () foo ()
>>> {
>>> int a_3;
>>> a_3_1 = 0;
>>> }
>>>
>>> correctly as it looks for the first '_' rather than the last. I'd have used
>>> sth like
>>>
>>> char *p = strrchr (ssa_token, '_');
>>> if (p)
>>> {
>>> var_name = new char [p - ssa_token + 1];
>>> memcpy (var_name, ssa_token, p - ssa_token);
>>> var_name[p - ssa_token] = '\0';
>>> ...
>>> version = atoi (p);
>>>
>>> in the patch you have two copies of the SSA name parsing code - you want to
>>> split that out into a function.
>>>
>>> There is one other feature missing for SSA name parsing (forget to mention
>>> that)
>>> which is parsing of default def SSA names. Those are for example used for
>>> parameters and currently dumped like
>>>
>>> foo (int i)
>>> {
>>> int D.1759;
>>> int _2;
>>>
>>> <bb 2>:
>>> _2 = i_1(D) + 1;
>>> return _2;
>>> }
>>>
>>> for int foo (int i) { return i + 1; }. Here 'i_1(D)' is the
>>> default-def of 'i' which
>>> in case of a parameter is the value at function start and in case of a
>>> non-parameter
>>> is simply "undefined". '(D)' is again somewhat awkward to parse but I
>>> guess we
>>> can cope with that ;) A default-def needs to be registered like
>>>
>>> arg = make_ssa_name_fn (cfun, lookup_name (id), ...);
>>> set_ssa_default_def (cfun, lookup_name (id), arg);
>>>
>>> "undefined" SSA names appear often in PHIs (and of course for parameters).
>>>
>>
>> This updated patch tries to parse default def ssa names
> Um does this emit error for cases like a_1() and a_(D) ?
> From the code it appears to me that parsing 'D' is made optional, so
> id_version() would be accepted.
> Perhaps have an else for the if (!strcmp("D", ...) that emits parse error ?
>
Right. Currently it gives ICE but we can handle it with better way.
> Btw for the following case:
> int a;
> int a_1;
> int x = a_1 + 1;
> What does a_1 refer to in "int x = a_1 + 1" ? the ssa-version of 'a'
> or the variable 'a_1' ?
> I think from the code it would refer to ssa-version of a ? However the
> reference looks
> ambiguous to me (since we also allow variables in non-ssa form).
>
we are guarding it with condition
if (TREE_CODE (c_parser_peek_token (parser)->value) == IDENTIFIER_NODE
&& !lookup_name (c_parser_peek_token (parser)->value))
so that shouldn't happen.
Thanks,
Prasad
> Thanks,
> Prathamesh
>>
>> Thanks,
>> Prasad
>>
>>> Thanks,
>>> Richard.
>>>
>>>> Thanks,
>>>> Prathamesh
>>>>>
>>>>> for testcase :
>>>>>
>>>>> void __GIMPLE () foo()
>>>>> {
>>>>> int a;
>>>>> bb_2:
>>>>> if (a > 4)
>>>>> goto bb_3;
>>>>> else
>>>>> goto bb_4;
>>>>> bb_3:
>>>>> a_1 = 55;
>>>>> goto bb_5;
>>>>>
>>>>> bb_4:
>>>>> a_2 = 99;
>>>>>
>>>>> bb_5:
>>>>> a_3 = __PHI (bb_3: a_1, bb_4: a_2);
>>>>> a_4 = a_3 + 3;
>>>>> return;
>>>>> }
>>>>>
>>>>> I am getting ssa dump as:
>>>>>
>>>>> /* Function foo (foo, funcdef_no=0, decl_uid=1744, cgraph_uid=0,
>>>>> symbol_order=0)*/
>>>>>
>>>>> void
>>>>> foo ()
>>>>> {
>>>>> bb_2:
>>>>> if (a_5 > 4)
>>>>> goto bb_3;
>>>>> else
>>>>> goto bb_4;
>>>>>
>>>>> bb_3:
>>>>> a_1 = 55;
>>>>> goto bb_5;
>>>>>
>>>>> bb_4:
>>>>> a_2 = 99;
>>>>>
>>>>> a_3 = __PHI (bb_3: a_1, bb_4: a_2)
>>>>> bb_5:
>>>>> a_4 = a_3 + 3;
>>>>> return;
>>>>>
>>>>> }
>>>>>
>>>>>> Richard.
>>>>>>
>>>>>>> Thanks,
>>>>>>> Prathamesh
>>>>>>>>
>>>>>>>> Richard.
>>>>>>>>
>>>>>>>>>
>>>>>>>>>> Richard.
>>>>>>>>>>
>>>>>>>>>>>Dave
>>>>>>>>>>
>>>>>>>>>>
