This is an archive of the discontinued LLVM Phabricator instance.

[Polly] Introduce isl_conv:: isl++ classes that automatically convert to old C classes
AbandonedPublic

Authored by grosser on Jul 21 2017, 12:59 PM.

Details

Summary

... and use these classes to port the ScopInfo.h interface to C++. This allows
us to return C++ objects everywhere in the interface, without having to convert
all C users at a time. As a result, new code can use C++ without the need for
std::give (or the new std::manage) calls all over the place, while old code
still works.

The isl++ interface on purpose does not allow for conversions back to isl C
types as there is a risk that automatic conversions might cause difficult to
detect memory leaks. The isl_conv:: classes allow such conversions, but are
expected to only be used at interface boundaries to avoid the need of isl++
conversion functions all over the place. After we moved to the C++ interface,
this conversion layer is expected to be removed again.

To make this patch compile, we have to remove some "auto" declarations and
replace them with specific types. This is a good direction anyhow, as LLVM's
coding style suggests to not use auto besides in obvious situations.

Event Timeline

grosser created this revision.Jul 21 2017, 12:59 PM
grosser updated this revision to Diff 107702.Jul 21 2017, 1:03 PM

Add missing conversion class.

Meinersbur edited edge metadata.Jul 21 2017, 1:44 PM

[Opinion] I don't see the problem with converting the C-style interface one-by-one to isl++. There is a mechanical conversion for legacy users, e.g.
getDomain() to getDomain().release(). For the parts already primarily using isl++, give(getDomain().release()) collapses to getDomain().

With isl_conv we still need to go through all users to eventually get rid of it again. Until then, we just have increased the complexity and chances for memory errors significantly, something the C++ interface was trying to improve.

If you really need this class, please don't take too long to remove it again. Can you give a time frame for when it will be gone?

include/polly/Support/ISLConversion.h
23

[Style] UPPER_CASE is usually reserved for macros. Template arguments in LLVM usually are CamelCase.

lib/Transform/Simplify.cpp
175–176

give removed for getDomain(), but not for getContext()?

If you need to touch a significant amount of users for this patch anyway, I don't see the purpose.

grosser abandoned this revision.Jul 21 2017, 3:21 PM

OK, you are probably right that this is not needed. Overall, there are around 200 calls to change. I will just class by class directly to the new interface.