Re: [GSoC] generation of Gimple loops with empty bodies

[Roman Gareev <gareevroman at gmail dot com>]

> I would use -x to see the headers. This should tell you if it is defined
> or only used there.

It gives the following output:

roman@roman-System-Product-Name:~/isl-0.12.2/lib$ objdump -x
libisl.so.10.2.2 | grep isl_ast_expr_get_val
0000000000060380 g     F .text 0000000000000053
isl_ast_expr_get_val

> Sorry, but this still needs to be debugged. :-( I am too busy to do it
> myself, so i am afraid you may just need to dig into it yourself. :-(

I've found the reason of this error. Somehow cc1 used an older version
of isl provided by the package from the standard repository in spite
of manually selected paths to isl 0.12.2. I think it is better to move
on and not to concentrate on this.

>> +#include "graphite-htab.h"
>
> Is this still needed?

No, this is redundant.

>> +/* IVS_PARAMS_MAP maps ISL's scattering and parameter identifiers
>> +   to corresponding trees.  */
>> +typedef struct ivs_params {
>> +  std::map<isl_id *, tree, id_comp> ivs_params_map;
>
> What about calling it isl_id_to_tree?

Maybe it would be better to call it tree_from_isl_id (because it would
help to avoid confusion with definitions from isl)?

>> +  sese region;
>
> Is this region needed?

No, this is redundant.

>> +   We create a new if region protecting the loop to be executed, if
>> +   the execution count is zero (lower bound > upper bound). In this case,
>> +   it is subsequently removed by dead code elimination.  */
>
>
> The last sentence is unclear. Are you saying the loop will be removed or
> that the condition will be removed in case it is always true. Also, what
> about cases where the condition remains?

I wanted to say that the loop will be removed. Maybe it would be
better to remove the last sentence at all (because the previous
sentence already explains the motivation)?

> Those changes are unrelated and are part of the other patch, right?

Yes, these changes were made in the revision under No. 212361.

>> +  ip.region = region;
>
> Is this needed? ip.region seems unused.

Yes, this is redundant.


I tried to incorporate all your comments in the attached patch.

--
                                   Cheers, Roman Gareev.

Attachment: patch.txt
Description: Text document