This is an archive of the discontinued LLVM Phabricator instance.

[WIP] Print Loop* associated with isl_ast_for
Needs ReviewPublic

Authored by dpeixott on Sep 9 2014, 12:18 PM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Summary

First patch to keep Loop* associated with its corresponding
isl_ast_for_node.

Diff Detail

Event Timeline

dpeixott updated this revision to Diff 13487.Sep 9 2014, 12:18 PM
dpeixott retitled this revision from to [WIP] Print Loop* associated with isl_ast_for.
dpeixott updated this object.
jdoerfert edited edge metadata.Sep 9 2014, 12:56 PM

Minor comments and questions.

include/polly/CodeGen/IslAst.h
49

Any reason not to use a unordered small set? (Doesn't really matter, I'm just curious)

142

When do we need this specialization? Shouldn't we always handle the general (LoopSet) case?

lib/CodeGen/IslAst.cpp
274

drop the {}

dpeixott added inline comments.Sep 9 2014, 1:30 PM
include/polly/CodeGen/IslAst.h
49

I initially used an unordered set, but switched to the ordered set to keep the output deterministic. Otherwise, we could print out the loops in a different order depending on their pointer values when iterating through the set.

142

Good point. I will remove it. It was for lazy programmers that didn't want to deal with the reality of multiple loops.

lib/CodeGen/IslAst.cpp
274

Ah, is that what the linter was flagging? I got confused because it was pointing to the top of the file. Will fix it up.

responses

include/polly/CodeGen/IslAst.h
49

I see the merrit of deterministic output here but it might not be needed here, however I do not want to argue against it.

lib/CodeGen/IslAst.cpp
274

It was not (some space in the setvector was), it is just our/llvm coding style for one statement conditionals/loops.

jdoerfert resigned from this revision.Sep 27 2015, 6:04 PM
jdoerfert removed a reviewer: jdoerfert.

Doesn't seem to be needed anymore.