This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [gimplefe] hacking pass manager


Thanks,
Prasad


On 29 July 2016 at 06:56, Prathamesh Kulkarni
<prathamesh.kulkarni@linaro.org> wrote:
> On 29 July 2016 at 00:01, Prasad Ghangal <prasad.ghangal@gmail.com> wrote:
>> On 27 July 2016 at 14:22, Richard Biener <richard.guenther@gmail.com> wrote:
>>> On Tue, Jul 26, 2016 at 11:38 PM, Prathamesh Kulkarni
>>> <prathamesh.kulkarni@linaro.org> wrote:
>>>> On 27 July 2016 at 00:20, Prasad Ghangal <prasad.ghangal@gmail.com> wrote:
>>>>> On 20 July 2016 at 18:28, Richard Biener <richard.guenther@gmail.com> wrote:
>>>>>> On Wed, Jul 20, 2016 at 1:46 PM, Prathamesh Kulkarni
>>>>>> <prathamesh.kulkarni@linaro.org> wrote:
>>>>>>> On 20 July 2016 at 11:34, Richard Biener <richard.guenther@gmail.com> wrote:
>>>>>>>> On Tue, Jul 19, 2016 at 10:09 PM, Prasad Ghangal
>>>>>>>> <prasad.ghangal@gmail.com> wrote:
>>>>>>>>> On 19 July 2016 at 11:04, Richard Biener <richard.guenther@gmail.com> wrote:
>>>>>>>>>> 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.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> 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
>>>>>>>>>>
>>>>>>>>>>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]