This is the mail archive of the 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: [GSoC] generation of Gimple code from isl_ast_node_user

> No, no idea. To my understanding the entry block should not even appear
> within a scop (see build_scops, where we only start detecting scops at
> the successor of the entry_block). Maybe we replace this with an assert
> to get a good error message in case I have missed something.

Yes, I think this would be a good solution at this moment.

> I think this region is actually not needed. At the place where you need
> it, you have a pbb available, from which you can obtain a pointer to the
> surrounding scop and from this you can obtain this region itself.

Yes, this is redundant.

> This is a pure style change which seems unrelated. Also, is the original
> line really too long? I may have miscounted, but it seems to fit
> exactly.

If I am not mistaken, lines should be limited to 80 characters,
according to conventions, which are mentioned here This line contains 81

> In polly we just create an iv_map from [0, max-loop-depth-within-scop], so
> it contains at most as many elements as the maximal loop depth. Your map
> is unnecessarily large, as it contains all loops in the function.
> If we can avoid this immediately, e.g. by indexing according to the
> loop-depths that would be great. On the other side, if the existing
> functions require this, I propose to not touch this area, but to add a
> fixme that documents this issue. We can solve it after having removed
> the CLooG codegen.

If I am not mistaken, the existing function doesn't require that
iv_map contains at most as many elements as the maximal loop depth.
(If we consider chrec_apply_map (I think that it is the only function,
which works with iv_map), we'll see that it calls chrec_apply when the
expression is not NULL. In our case, we push NULL_TREEs into iv_map to
extend its size to the maximal loop depth. They aren't considered,
because tree NULL_TREE is equivalent to NULL, according to tree.h)

Maybe we could use a number of the innermost loop that contains the
basic block gbb. If I am not mistaken, outer loops considered in
build_iv_mapping have smaller numbers than it. What do you think about

> I was just briefly looking the code to remind me what this
> bb_pbb_mapping hash table is about, but could not find the reason it is
> needed. Do you know why it is needed?
> Is it necessary for this patch? Or did you just copy it from the
> previous code generation?

Yes, I've forgotten to remove this. If I am not mistaken,
bb_pbb_mapping is used for checking for the loop parallelism in

> Why do you need to first push NULL_TREEs into this vec, which will be
> overwritten right after?

In the current implementation, we'll get the error âinternal compiler
error: in operator[], at vec.h:736â, if we try to assign a value to
iv_map via [] and remove initial pushing of NULL_TREEs. We could push
new values into iv_map in build_iv_mapping, but I am not sure if there
are cases, which require special processing (for example, NULL_TREEs
should be pushed into iv_map before the next old_loop->num. Possibly,
there are no such cases.).

> It is unclear how this patches have been tested. Can you elaborate?

I've compared the result of Gimple code generation (I've attached them
to the letter) for the following examples:

main (int n, int *a)
  int i;

  for (i = n; i < 100; i++)
    a[i] = i;

 return 0;

main (int 0, int *a)
  int i;

  for (i = n; i < 100; i++)
    a[i] = i;

 return 0;

> Also, we need to find a way to test this in gcc itself, possibly by
> running test cases that already work with both the cloog and the isl ast
> generator.

Maybe we could choose the strategy, which was used in
(The result of the transformed function is compared to the assumed
value and the abort function is called in case of inequality.) What do
you think about this?

                                   Cheers, Roman Gareev.

Attachment: ChangeLog_entry.txt
Description: Text document

Attachment: cloog-1.txt
Description: Text document

Attachment: cloog-2.txt
Description: Text document

Attachment: isl-1.txt
Description: Text document

Attachment: isl-2.txt
Description: Text document

Attachment: patch.txt
Description: Text document

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