This is an archive of the discontinued LLVM Phabricator instance.

[Polly] Use isl C++ foreach implementation
ClosedPublic

Authored by grosser on Mar 5 2017, 8:19 AM.

Details

Summary

This commit switches Polly over to the isl::Obj::foreach_* implementation,
which is part of the new isl bindings and follows the foreach pattern
established in Polly by Michael Kruse.

The original isl C function:

isl_stat isl_union_set_foreach_set(__isl_keep isl_union_set *uset,
    isl_stat (*fn)(__isl_take isl_set *set, void *user), void *user);

which required the user to define a static callback function to which all
interesting parameters are passed via a 'void *' user-pointer, is on the
C++ side available as a function that takes a std::function<>, which can
carry any additional arguments without the need for a user pointer:

stat UnionSet::foreach_set(std::function<stat(set)> &&fn) const;

The following code illustrates the use of the new C++ interface:

auto Lambda = [=, &Result](isl::set Set) -> isl::stat {
  auto Shifted = shiftDimension(Set, Pos, Amount);
  Result = Result.add(Shifted);
  return isl::stat::ok;
}

UnionSet.foreach_set(Lambda);

Polly had some specialized foreach functions which did not require the lambdas
to return a status flag. We remove these functions in this commit to move
Polly completely over to the new isl interface. We may in the future discuss
if functors without return values can be supported easily.

Diff Detail

Repository
rL LLVM

Event Timeline

grosser created this revision.Mar 5 2017, 8:19 AM
grosser retitled this revision from Use isl C++ foreach implementation to [Polly] Use isl C++ foreach implementation.Mar 5 2017, 8:22 AM
grosser edited the summary of this revision. (Show Details)
Meinersbur edited edge metadata.Mar 6 2017, 2:39 AM

I'd prefer having an iterator implementation for the C++ bindings instead of a callback one. Since it requires support for isl, we previously decided to delay it to keep the bindings simple until the bindings are committed.

Michael, this is a very good point. I wrote this to make sure we have a minimal and complete implementation of these bindings to show that the _base_ we propose for inclusion is mature enough to handle foreach generation. I agree that an iterator interface would be preferable, but is likely more difficult to implement. I agree with you that it would be preferable to leave this out from the initial bindings and have afterwards a dedicated discussion which focused on the iterator interface. I will leave this out in the first proposal.

Hi Michael,

it seems Sven feels strongly about also providing a foreach based interface. As such an interface will be anyhow part of the isl C++ bindings, I wonder if it makes sense to commit this patch for now and then later move to the iterator based interface, when available? Or would you like me to abondon this commit and wait for your proposal of an iterator based interface? I am fine with any of the two.

Whatever you think makes more sense is also ok for me.

This revision was automatically updated to reflect the committed changes.