Re: [Gcc4cli-devel] Source location patch #2

[Thierry L. <thi...@in...>]

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
>