Menu
▾
▴
gcc4cli-devel
|
From: Thierry L. <thi...@in...> - 2009-12-11 15:56:49
Attachments:
gcc4cli_line_info-2.patch
|
Hi all, Here is the second version of the patch for .line info in CIL generated code. Feel free to comment. If you have no other blocking points, I'll commit it as soon as I have my account on sourceware.org (which seems to last a bit). Regards, Thierry. |
|
From: Andrea C. O. <and...@gm...> - 2009-12-11 18:16:40
|
I do not have any new remark :) BTW I do not understand why, but I have to approve all your emails, I did mistyped your email address? I set the mailing list to always accept mails from you, we will see. Andrea On Fri, Dec 11, 2009 at 4:56 PM, Thierry LAFAGE <thi...@in...> wrote: > Hi all, > > Here is the second version of the patch for .line info in CIL generated > code. > Feel free to comment. > If you have no other blocking points, I'll commit it as soon as I have my > account on sourceware.org (which seems to last a bit). > > Regards, > Thierry. |
|
From: Erven R. <erv...@in...> - 2009-12-13 15:25:10
|
Andrea Carlo Ornstein a écrit : > I do not have any new remark :) > > BTW I do not understand why, but I have to approve all your emails, > I did mistyped your email address? We have many email addresses here: lo...@ir... fir...@ir... fir...@in... Maybe you registered Thierry with one and his mailer used another one? Thanks for the feedback on the patches, anyway. :-) Ciao, -- Erven. > I set the mailing list to always accept mails from you, we will see. > > Andrea > > On Fri, Dec 11, 2009 at 4:56 PM, Thierry LAFAGE <thi...@in...> wrote: >> Hi all, >> >> Here is the second version of the patch for .line info in CIL generated >> code. >> Feel free to comment. >> If you have no other blocking points, I'll commit it as soon as I have my >> account on sourceware.org (which seems to last a bit). >> >> Regards, >> Thierry. > > ------------------------------------------------------------------------------ > Return on Information: > Google Enterprise Search pays you back > Get the facts. > http://p.sf.net/sfu/google-dev2dev > _______________________________________________ > Gcc4cli-devel mailing list > Gcc...@li... > https://lists.sourceforge.net/lists/listinfo/gcc4cli-devel |
|
From: Thierry L. <thi...@in...> - 2009-12-14 08:20:24
|
Ciao, In fact, the first time you had to approve because I use @inria instead of @irisa (and I was registered as @irisa). Then I changed my account on the list to be registered as @inria. Then, this time an approval was needed because my post was too large... Regards, Thierry Lafage. Le 13/12/2009 16:24, Erven Rohou a écrit : > Andrea Carlo Ornstein a écrit : > >> I do not have any new remark :) >> >> BTW I do not understand why, but I have to approve all your emails, >> I did mistyped your email address? >> > We have many email addresses here: > lo...@ir... > fir...@ir... > fir...@in... > Maybe you registered Thierry with one and his mailer used another one? > > Thanks for the feedback on the patches, anyway. :-) > > Ciao, > > -- > Erven. > > > >> I set the mailing list to always accept mails from you, we will see. >> >> Andrea >> >> On Fri, Dec 11, 2009 at 4:56 PM, Thierry LAFAGE<thi...@in...> wrote: >> >>> Hi all, >>> >>> Here is the second version of the patch for .line info in CIL generated >>> code. >>> Feel free to comment. >>> If you have no other blocking points, I'll commit it as soon as I have my >>> account on sourceware.org (which seems to last a bit). >>> >>> Regards, >>> Thierry. >>> >> ------------------------------------------------------------------------------ >> Return on Information: >> Google Enterprise Search pays you back >> Get the facts. >> http://p.sf.net/sfu/google-dev2dev >> _______________________________________________ >> Gcc4cli-devel mailing list >> Gcc...@li... >> https://lists.sourceforge.net/lists/listinfo/gcc4cli-devel >> > ------------------------------------------------------------------------------ > Return on Information: > Google Enterprise Search pays you back > Get the facts. > http://p.sf.net/sfu/google-dev2dev > _______________________________________________ > Gcc4cli-devel mailing list > Gcc...@li... > https://lists.sourceforge.net/lists/listinfo/gcc4cli-devel > |
|
From: Thierry L. <thi...@in...> - 2009-12-14 09:08:36
|
Hi Gabriele, Thanks for your remarks. What kind of coding guidelines do you think of? I can fix them before ciing. Thierry Lafage. Le 13/12/2009 20:36, Gabriele Svelto a écrit : > 2009/12/11 Thierry LAFAGE<thi...@in...>: > >> Hi all, >> >> Here is the second version of the patch for .line info in CIL generated >> code. >> Feel free to comment. >> If you have no other blocking points, I'll commit it as soon as I have my >> account on sourceware.org (which seems to last a bit). >> >> Regards, >> Thierry. >> > I've looked quickly at the patch and it seems good to me. I just > noticed a couple of places here and there were you didn't cope with > GCC coding guidelines but those can be fixed later. > > Gabriele > |
|
From: Gabriele S. <gab...@gm...> - 2009-12-14 09:30:29
|
2009/12/14 Thierry LAFAGE <thi...@in...>:
> Hi Gabriele,
>
> Thanks for your remarks. What kind of coding guidelines do you think of?
> I can fix them before ciing.
>
> Thierry Lafage.
Some lines are longer than 80 characters, those should be split on two
lines (there are many places were this is done so you can follow the
style used in those lines). I also noticed a couple of places were you
have left a multi-line statement without putting brackets around it,
for example:
if (TREE_CODE (node) == REALPART_EXPR)
stmt = cil_build_stmt_arg (CIL_LDFLDA,
cil_get_builtin_complex_real_fld (type));
Should be:
if (TREE_CODE (node) == REALPART_EXPR)
{
stmt = cil_build_stmt_arg (CIL_LDFLDA,
cil_get_builtin_complex_real_fld (type));
}
BTW the same line has 8 spaces in front of the new statement, in GCC
you should use 2, 4 or 6 spaces when indenting and insert a tabulator
every 8 spaces. Finally a space is always left between the name of a
function and the arguments, e.g.:
cil_set_locus(stmt, sloc);
Should become:
cil_set_locus (stmt, sloc);
I know it's annoying, GCC coding guidelines are very quirky but it's
better to stick to them because it simplifies merges with the
mainline. You will get used to them eventually ;-)5
Just a final remark which you might find interesting for your future
work. In our gimple-to-CIL phase we turn GIMPLE nodes into TREE nodes
for simplicity because our original work was done in a time when this
was not necessary because GIMPLE statements were 'special' tree nodes,
now they are specific structures on the other hand. However even if
GIMPLE nodes have their own structures the functionality used for
turning them into trees was left into GCC because nobody wanted to
rewrite the pass (expand) that turned GIMPLE code into RTL code (the
lower-level GCC IR). I haven't been able to follow closely the latest
GCC development but it is entirely possible that in version 4.5 that
pass will have been rewritten and GIMPLE-to-tree functionality
removed. We will have to rewrite the gimple2cil to accomodate for this
in the future and the source location information will also have to
come from GIMPLE so I suggest that you investigate (starting from
gimple_location() and friends) if your work can be adapted to it. I
believe it should be possible and fairly easy; in fact when we will
have to deal with only GIMPLE nodes the whole pass should be simpler
(and we will need a much better stack-promotion pass!). Ciao!
Gabriele
|
|
From: Thierry L. <thi...@in...> - 2009-12-14 09:43:48
|
Ok. Thanks.
I'll try to comply with those quite annoying formatting rules (have to
better tune my IDE).
Concerning location information directly from gimple, I hope that this
information will be correct and accurate in 4.5 since it does not seem
to be the case at the moment (for this reason I had to pick it into the
tree representation). If the gimple2cil pass is rewritten to comply with
version 4.5 then I'll make my code comply to it too.
Thierry Lafage.
Le 14/12/2009 10:29, Gabriele Svelto a écrit :
> 2009/12/14 Thierry LAFAGE<thi...@in...>:
>
>> Hi Gabriele,
>>
>> Thanks for your remarks. What kind of coding guidelines do you think of?
>> I can fix them before ciing.
>>
>> Thierry Lafage.
>>
> Some lines are longer than 80 characters, those should be split on two
> lines (there are many places were this is done so you can follow the
> style used in those lines). I also noticed a couple of places were you
> have left a multi-line statement without putting brackets around it,
> for example:
>
> if (TREE_CODE (node) == REALPART_EXPR)
> stmt = cil_build_stmt_arg (CIL_LDFLDA,
> cil_get_builtin_complex_real_fld (type));
>
> Should be:
>
> if (TREE_CODE (node) == REALPART_EXPR)
> {
> stmt = cil_build_stmt_arg (CIL_LDFLDA,
> cil_get_builtin_complex_real_fld (type));
> }
>
> BTW the same line has 8 spaces in front of the new statement, in GCC
> you should use 2, 4 or 6 spaces when indenting and insert a tabulator
> every 8 spaces. Finally a space is always left between the name of a
> function and the arguments, e.g.:
>
> cil_set_locus(stmt, sloc);
>
> Should become:
>
> cil_set_locus (stmt, sloc);
>
> I know it's annoying, GCC coding guidelines are very quirky but it's
> better to stick to them because it simplifies merges with the
> mainline. You will get used to them eventually ;-)5
>
> Just a final remark which you might find interesting for your future
> work. In our gimple-to-CIL phase we turn GIMPLE nodes into TREE nodes
> for simplicity because our original work was done in a time when this
> was not necessary because GIMPLE statements were 'special' tree nodes,
> now they are specific structures on the other hand. However even if
> GIMPLE nodes have their own structures the functionality used for
> turning them into trees was left into GCC because nobody wanted to
> rewrite the pass (expand) that turned GIMPLE code into RTL code (the
> lower-level GCC IR). I haven't been able to follow closely the latest
> GCC development but it is entirely possible that in version 4.5 that
> pass will have been rewritten and GIMPLE-to-tree functionality
> removed. We will have to rewrite the gimple2cil to accomodate for this
> in the future and the source location information will also have to
> come from GIMPLE so I suggest that you investigate (starting from
> gimple_location() and friends) if your work can be adapted to it. I
> believe it should be possible and fairly easy; in fact when we will
> have to deal with only GIMPLE nodes the whole pass should be simpler
> (and we will need a much better stack-promotion pass!). Ciao!
>
> Gabriele
>
|
|
From: Gabriele S. <gab...@gm...> - 2009-12-14 09:57:37
|
2009/12/14 Thierry LAFAGE <thi...@in...>: > Ok. Thanks. > I'll try to comply with those quite annoying formatting rules (have to > better tune my IDE). Yes, it's the easiest thing to avoid problems, also ensure that the editor always trims trailing spaces when saving files, there a few places in GCCs were trailing spaces were left and nobody modified them for a long time in order not to make merges harder (yeah, it's completely crazy). > Concerning location information directly from gimple, I hope that this > information will be correct and accurate in 4.5 since it does not seem to be > the case at the moment (for this reason I had to pick it into the tree > representation). If the gimple2cil pass is rewritten to comply with version > 4.5 then I'll make my code comply to it too. I have checked in GCC trunk's svn history and apparently gimple_to_tree() has already been removed so 4.5 won't have the GIMPLE->TREE->RTL scheme anymore (GIMPLE->TREE->CLI). I was wondering how they do get proper location information on the other hand because the TREE from which you extract the source location has been built from GIMPLE statements. Looking at the old code apparently source location information was converted from GIMPLE back into the TREE representation at the bottom of the gimple_to_tree() function in cfgexpand.c. Here's the relevant snippet: /* Set EXPR_LOCATION in all the embedded expressions. */ loc = gimple_location (stmt); walk_tree (&t, set_expr_location_r, (void *) &loc, NULL); Ciao! Gabriele |
|
From: Thierry L. <thi...@in...> - 2010-01-18 17:31:54
|
Hi, I've just commited the patch we discussed concerning source location information in our branch (including fixes wrt. coding guidelines). Regards, Thierry Lafage. Gabriele Svelto a écrit : > 2009/12/14 Thierry LAFAGE <thi...@in...>: > >> Ok. Thanks. >> I'll try to comply with those quite annoying formatting rules (have to >> better tune my IDE). >> > > Yes, it's the easiest thing to avoid problems, also ensure that the > editor always trims trailing spaces when saving files, there a few > places in GCCs were trailing spaces were left and nobody modified them > for a long time in order not to make merges harder (yeah, it's > completely crazy). > > >> Concerning location information directly from gimple, I hope that this >> information will be correct and accurate in 4.5 since it does not seem to be >> the case at the moment (for this reason I had to pick it into the tree >> representation). If the gimple2cil pass is rewritten to comply with version >> 4.5 then I'll make my code comply to it too. >> > > I have checked in GCC trunk's svn history and apparently > gimple_to_tree() has already been removed so 4.5 won't have the > GIMPLE->TREE->RTL scheme anymore (GIMPLE->TREE->CLI). I was wondering > how they do get proper location information on the other hand because > the TREE from which you extract the source location has been built > from GIMPLE statements. Looking at the old code apparently source > location information was converted from GIMPLE back into the TREE > representation at the bottom of the gimple_to_tree() function in > cfgexpand.c. Here's the relevant snippet: > > /* Set EXPR_LOCATION in all the embedded expressions. */ > loc = gimple_location (stmt); > walk_tree (&t, set_expr_location_r, (void *) &loc, NULL); > > Ciao! > > Gabriele > |
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X