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


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

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]