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

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
>>>>>>>>
>>>>>>>>
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 1a55b9a..1d2fc2d 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -66,6 +66,9 @@ along with GCC; see the file COPYING3.  If not see
 #include "gimple-pretty-print.h"
 #include "tree-ssa.h"
 #include "pass_manager.h"
+#include "tree-ssanames.h"
+#include "gimple-ssa.h"
+#include "tree-dfa.h"
 
 /* We need to walk over decls with incomplete struct/union/enum types
    after parsing the whole translation unit.
@@ -1421,7 +1424,7 @@ static void c_parser_gimple_if_stmt (c_parser *, gimple_seq *);
 static void c_parser_gimple_switch_stmt (c_parser *, gimple_seq *);
 static void c_parser_gimple_return_stmt (c_parser *, gimple_seq *);
 static void c_finish_gimple_return (location_t, tree);
-
+static c_expr c_parser_parse_ssa_names (c_parser *);
 
 /* Parse a translation unit (C90 6.7, C99 6.9).
 
@@ -1666,7 +1669,7 @@ c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok,
   location_t here = c_parser_peek_token (parser)->location;
   bool gimple_body_p = false;
   opt_pass *pass = NULL;
-  bool startwith_p;
+  bool startwith_p = false;
 
   if (static_assert_ok
       && c_parser_next_token_is_keyword (parser, RID_STATIC_ASSERT))
@@ -1723,7 +1726,6 @@ c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok,
       if (kw_token->keyword == RID_GIMPLE)
 	{
 	  gimple_body_p = true;
-	  startwith_p = false;
 	  c_parser_consume_token (parser);
 	  c_parser_gimple_pass_list (parser, &pass, &startwith_p);
 	  c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, 
@@ -18145,7 +18147,7 @@ c_parser_parse_gimple_body (c_parser *parser)
   location_t loc1 = c_parser_peek_token (parser)->location;
   seq = NULL;
   body = NULL;
-  
+  init_tree_ssa (cfun);
   return_p = c_parser_gimple_compound_statement (parser, &seq);
 
   if (!return_p)
@@ -18173,7 +18175,6 @@ c_parser_parse_gimple_body (c_parser *parser)
   cfun->curr_properties = PROP_gimple_any;
   if (flag_gdebug)
     debug_gimple_seq (seq);
-  init_tree_ssa (cfun);
   return;
 }
 
@@ -18361,7 +18362,7 @@ c_parser_gimple_expression (c_parser *parser, gimple_seq *seq)
 
       gcall *call_stmt;
       /* Gimplify internal functions. */
-      tree arg;
+      tree arg = NULL_TREE;
       vec<tree> vargs = vNULL;
 
       if (c_parser_next_token_is (parser, CPP_OPEN_PAREN))
@@ -18386,7 +18387,7 @@ c_parser_gimple_expression (c_parser *parser, gimple_seq *seq)
 	    }
 	  else
 	    {
-	      arg = c_parser_gimple_unary_expression (parser).value;
+	      arg = c_parser_parse_ssa_names (parser).value;
 	      vargs.safe_push (arg);
 	    }
 	}
@@ -18481,7 +18482,7 @@ c_parser_gimple_binary_expression (c_parser *parser, enum tree_code *subcode)
     sp--;								      \
   } while (0)
   stack[0].loc = c_parser_peek_token (parser)->location;
-  stack[0].expr = c_parser_cast_expression (parser, NULL);
+  stack[0].expr = c_parser_gimple_unary_expression (parser);
   stack[0].prec = PREC_NONE;
   sp = 0;
   enum c_parser_prec oprec;
@@ -18601,7 +18602,7 @@ c_parser_gimple_binary_expression (c_parser *parser, enum tree_code *subcode)
     }
   sp++;
   stack[sp].loc = binary_loc;
-  stack[sp].expr = c_parser_cast_expression (parser, NULL);
+  stack[sp].expr = c_parser_gimple_unary_expression (parser);
   stack[sp].prec = oprec;
   stack[sp].op = *subcode;
 out:
@@ -18618,6 +18619,10 @@ static c_expr
 c_parser_gimple_unary_expression (c_parser *parser)
 {
   struct c_expr ret, op;
+  if (TREE_CODE (c_parser_peek_token (parser)->value) == IDENTIFIER_NODE
+      && !lookup_name (c_parser_peek_token (parser)->value))
+    return c_parser_parse_ssa_names (parser);
+
   location_t op_loc = c_parser_peek_token (parser)->location;
   location_t exp_loc;
   location_t finish;
@@ -18676,6 +18681,56 @@ c_parser_gimple_unary_expression (c_parser *parser)
     }
 }
 
+/* Parse ssa names */
+
+static c_expr
+c_parser_parse_ssa_names (c_parser *parser)
+{
+  tree id = NULL_TREE;
+  c_expr ret;
+  char *var_name, *var_version, *token;
+  ret.original_code = ERROR_MARK;
+  ret.original_type = NULL;
+  const char *ssa_token = IDENTIFIER_POINTER (c_parser_peek_token (parser)->value);
+  token = new char [strlen (ssa_token)];
+  strcpy (token, ssa_token);
+  var_version = strrchr (token, '_');
+  if (var_version)
+    {
+      var_name = new char[var_version - token + 1];
+      memcpy (var_name, token, var_version - token);
+      var_name[var_version - token] = '\0';
+      id = get_identifier (var_name);
+      if (lookup_name (id))
+	{
+	  var_version++;
+	  unsigned int version;
+	  version = atoi (var_version);
+	  if (var_version && version)
+	    {
+	      ret.value = NULL_TREE;
+	      if (version < num_ssa_names)
+		ret.value = ssa_name (version);
+	      if (!ret.value)
+		ret.value = make_ssa_name_fn (cfun, lookup_name (id), gimple_build_nop (), version);
+	      c_parser_consume_token (parser);
+	    }
+	}
+    }
+  if (c_parser_next_token_is (parser, CPP_OPEN_PAREN))
+    {
+      c_parser_consume_token (parser);
+      if (!strcmp ("D",IDENTIFIER_POINTER (c_parser_peek_token (parser)->value)))
+	{
+	  set_ssa_default_def (cfun, lookup_name (id), ret.value);
+	  c_parser_consume_token (parser);
+	}
+      if (!c_parser_require (parser, CPP_CLOSE_PAREN, "expected %<)%>"))
+	return ret;
+    }
+  return ret;
+}
+
 /*Parser gimple postfix expression*/
 
 static struct c_expr 
@@ -18868,7 +18923,7 @@ c_parser_gimple_pass_list_params (c_parser *parser, opt_pass **pass)
 
       if (c_parser_next_token_is (parser, CPP_STRING))
 	{
-	  const char *name = TREE_STRING_POINTER(c_parser_peek_token (parser)->value);
+	  const char *name = TREE_STRING_POINTER (c_parser_peek_token (parser)->value);
 	  c_parser_consume_token (parser);
 	  new_pass = g->get_passes ()->get_pass_by_name (name);
 
@@ -19205,7 +19260,7 @@ c_finish_gimple_return (location_t loc, tree retval)
 		  "declared here");
 	}
     }
-  else if (TREE_CODE (valtype) != TREE_CODE (retval))
+  else if (TREE_CODE (valtype) != TREE_CODE (TREE_TYPE (retval)))
     {
       error_at
 	(xloc, "invalid conversion in return statement");
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index 8842cec..288e983 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -347,6 +347,7 @@ replace_loop_annotate (void)
 }
 
 /* Lower internal PHI function from GIMPLE FE */
+
 static void 
 lower_phi_internal_fn ()
 {
@@ -356,8 +357,8 @@ lower_phi_internal_fn ()
   gphi *phi_node;
   gimple *stmt;
   int len, capacity;
-  /* After edge creation, handle __PHI function from GIMPLE FE */
 
+  /* After edge creation, handle __PHI function from GIMPLE FE */
   FOR_EACH_BB_FN (bb, cfun)
     {
       for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
@@ -368,12 +369,10 @@ lower_phi_internal_fn ()
 
 	  if (gimple_call_internal_p (stmt) && gimple_call_internal_fn (stmt) == IFN_PHI)
 	    {
-	     gsi_remove (&gsi, true);
+	      gsi_remove (&gsi, true);
 	      int i;
 	      lhs = gimple_call_lhs (stmt);
-	      
 	      phi_node = create_phi_node (lhs, bb);
-	      
 	      for (i = 0; i < gimple_call_num_args (stmt); ++i)
 		{
 		  tree arg = gimple_call_arg (stmt, i);
@@ -7625,7 +7624,7 @@ dump_function_to_file (tree fndecl, FILE *file, int flags)
 	for (ix = 1; ix < num_ssa_names; ++ix)
 	  {
 	    tree name = ssa_name (ix);
-	    if (!virtual_operand_p (name))
+	    if (name && !SSA_NAME_VAR (name))
 	      {
 		fprintf (file, "  ");
 		print_generic_expr (file, TREE_TYPE (name), flags);
diff --git a/gcc/tree-into-ssa.c b/gcc/tree-into-ssa.c
index 71150ac..46c47d4 100644
--- a/gcc/tree-into-ssa.c
+++ b/gcc/tree-into-ssa.c
@@ -1386,7 +1386,8 @@ rewrite_add_phi_arguments (basic_block bb)
 	  /* If we have pre-existing PHI its args may be different 
 	     vars than existing vars */
 	  argvar = gimple_phi_arg_def (phi, e->dest_idx);
-	  gcc_assert (!argvar || TREE_CODE (argvar) != SSA_NAME);
+	  if (argvar && TREE_CODE (argvar) == SSA_NAME)
+	    continue;
 	  if (!argvar)
 	    argvar = SSA_NAME_VAR (res);
 	  currdef = get_reaching_def (argvar);
diff --git a/gcc/tree-pretty-print.c b/gcc/tree-pretty-print.c
index 0760587..03ade22 100644
--- a/gcc/tree-pretty-print.c
+++ b/gcc/tree-pretty-print.c
@@ -2704,6 +2704,10 @@ dump_generic_node (pretty_printer *pp, tree node, int spc, int flags,
 	}
       pp_underscore (pp);
       pp_decimal_int (pp, SSA_NAME_VERSION (node));
+      if (SSA_NAME_IS_DEFAULT_DEF (node))
+	pp_string (pp, "(D)");
+      if (SSA_NAME_OCCURS_IN_ABNORMAL_PHI (node))
+	pp_string (pp, "(ab)");
       break;
 
     case WITH_SIZE_EXPR:
diff --git a/gcc/tree-ssanames.c b/gcc/tree-ssanames.c
index 91a8f97..3017b51 100644
--- a/gcc/tree-ssanames.c
+++ b/gcc/tree-ssanames.c
@@ -255,7 +255,7 @@ flush_ssaname_freelist (void)
    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, unsigned int version)
 {
   tree t;
   use_operand_p imm;
@@ -265,8 +265,18 @@ make_ssa_name_fn (struct function *fn, tree var, gimple *stmt)
 	      || 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;
+      if (version >= SSANAMES (fn)->length ())
+	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++;
diff --git a/gcc/tree-ssanames.h b/gcc/tree-ssanames.h
index c81b1a1..228f021 100644
--- a/gcc/tree-ssanames.h
+++ b/gcc/tree-ssanames.h
@@ -79,7 +79,7 @@ extern bool ssa_name_has_boolean_range (tree);
 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 *, unsigned int version = 0);
 extern void release_ssa_name_fn (struct function *, tree);
 extern bool get_ptr_info_alignment (struct ptr_info_def *, unsigned int *,
 				    unsigned int *);

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