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 ?
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).
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
>>>>>>>>>
>>>>>>>>>
