Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cpp: export id #18

Open
wants to merge 1 commit into
base: continious-integration
Choose a base branch
from
Open

cpp: export id #18

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion include/isl/id.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
extern "C" {
#endif

struct isl_id;
struct __isl_export isl_id;
typedef struct isl_id isl_id;

ISL_DECLARE_LIST(id)
Expand All @@ -23,7 +23,9 @@ __isl_give isl_id *isl_id_alloc(isl_ctx *ctx,
__isl_give isl_id *isl_id_copy(isl_id *id);
__isl_null isl_id *isl_id_free(__isl_take isl_id *id);

__isl_export
void *isl_id_get_user(__isl_keep isl_id *id);
__isl_export
__isl_keep const char *isl_id_get_name(__isl_keep isl_id *id);

__isl_give isl_id *isl_id_set_free_user(__isl_take isl_id *id,
Expand Down
3 changes: 3 additions & 0 deletions include/isl/set.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,13 @@ isl_bool isl_set_has_dim_id(__isl_keep isl_set *set,
enum isl_dim_type type, unsigned pos);
__isl_give isl_id *isl_set_get_dim_id(__isl_keep isl_set *set,
enum isl_dim_type type, unsigned pos);
__isl_export
__isl_give isl_set *isl_set_set_tuple_id(__isl_take isl_set *set,
__isl_take isl_id *id);
__isl_give isl_set *isl_set_reset_tuple_id(__isl_take isl_set *set);
__isl_export
isl_bool isl_set_has_tuple_id(__isl_keep isl_set *set);
__isl_export
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel these could be a separate commit. But let's worry about this when upstreaming.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is used for tests.

__isl_give isl_id *isl_set_get_tuple_id(__isl_keep isl_set *set);
__isl_give isl_set *isl_set_reset_user(__isl_take isl_set *set);

Expand Down
74 changes: 73 additions & 1 deletion interface/cpp.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ void cpp_generator::print_class(ostream &os, const isl_class &clazz)
print_ptr_decl(os, clazz);
osprintf(os, "\n");
print_methods_decl(os, clazz);
print_custom_public_decl(os, clazz);

osprintf(os, "};\n");
}
Expand Down Expand Up @@ -371,6 +372,31 @@ void cpp_generator::print_methods_decl(ostream &os, const isl_class &clazz)
print_method_group_decl(os, clazz, it->first, it->second);
}

/* Print declarations for custom members of a class "clazz" to "os", based on
* the class name.
*/
void cpp_generator::print_custom_public_decl(ostream &os,
const isl_class &clazz)
{
string cppname = type2cpp(clazz);

if ("id" == cppname) {
const char *declarations =
" inline id(isl::ctx ctx, const std::string &name);\n"
" inline id(isl::ctx ctx, const std::string &name,\n"
" void *usr,\n"
" void (*deleter)(void *) = nullptr);\n"
" inline id(isl::ctx ctx, void *usr,\n"
" void (*deleter)(void *) = nullptr);\n"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The functions you choose look good. I am slightly afraid that pretty printing them like this won't fly. At the very least, the first one could be declared as C function and normally exposed. This would make sure that isl ids are also available in python.

Not sure about the other two. Do they make any sense in python? I mean, do we want to pass references to other python objects as user point? (A deleter certainly does not make sense)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rather maintain a dict id->object separately. My recommendation would be to do so in C++, too. The only place where I need the user pointer is interacting with PPCG-generated schedule trees.

As a radical solution, we can hide the user pointer from C++ and python completely. If the user needs it, they can do access it by extracting a C isl_id and be thus "warned" that the underlying data is a C object.

" inline bool has_name() const;\n"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also just add an __isl_bool isl_id_has_name() method and export it.

" inline id set_free_user("
"void (*deleter)(void *)) const;\n"
" inline bool operator==(const id &other) const;\n"
" inline bool operator!=(const id &other) const;\n";
osprintf(os, "%s", declarations);
}
}

/* Print declarations for methods "methods" of name "fullname" in class "clazz"
* to "os".
*
Expand Down Expand Up @@ -427,6 +453,8 @@ void cpp_generator::print_class_impl(ostream &os, const isl_class &clazz)
print_ptr_impl(os, clazz);
osprintf(os, "\n");
print_methods_impl(os, clazz);
osprintf(os, "\n");
print_custom_methods_impl(os, clazz);
}

/* Print implementation of global factory function to "os".
Expand Down Expand Up @@ -563,6 +591,49 @@ void cpp_generator::print_methods_impl(ostream &os, const isl_class &clazz)
}
}

/* Print definitions for custom methods of class "clazz" to "os", based on the
* class name.
*/
void cpp_generator::print_custom_methods_impl(ostream &os,
const isl_class &clazz)
{
string name = type2cpp(clazz);
if ("id" == name) {
const char *definitions =
"id::id(isl::ctx ctx, const std::string &name) {\n"
" ptr = isl_id_alloc(ctx.get(), name.c_str(),\n"
" nullptr);\n"
"}\n\n"
"id::id(isl::ctx ctx, const std::string &name,\n"
" void *user, void (*deleter)(void *)) {\n"
" ptr = isl_id_alloc(ctx.get(), name.c_str(), user);\n"
" if (deleter)\n"
" ptr = isl_id_set_free_user(ptr, deleter);\n"
"}\n\n"
"id::id(isl::ctx ctx, void *user,\n"
" void (*deleter)(void *)) {\n"
" ptr = isl_id_alloc(ctx.get(), nullptr, user);\n"
" if (deleter)\n"
" ptr = isl_id_set_free_user(ptr, deleter);\n"
"}\n\n"
"bool id::has_name() const {\n"
" return isl_id_get_name(ptr) != nullptr;\n"
"}\n\n"
"id id::set_free_user("
"void (*deleter)(void *)) const {\n"
" auto res = isl_id_set_free_user(copy(), deleter);\n"
" return manage(res);\n"
"}\n\n"
"bool id::operator==(const isl::id &other) const {\n"
" return ptr == other.ptr;\n"
"}\n\n"
"bool id::operator!=(const isl::id &other) const {\n"
" return !operator==(other);\n"
"}\n\n";
osprintf(os, "%s", definitions);
}
}

/* Print definitions for methods "methods" of name "fullname" in class "clazz"
* to "os".
*
Expand Down Expand Up @@ -1023,7 +1094,8 @@ string cpp_generator::type2cpp(QualType type)
if (is_isl_stat(type))
return "isl::stat";

if (type->isIntegerType())
if (type->isIntegerType() || type->isVoidType() ||
type->isVoidPointerType())
return type.getAsString();

if (is_string(type))
Expand Down
2 changes: 2 additions & 0 deletions interface/cpp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class cpp_generator : public generator {
void print_destructor_decl(ostream &os, const isl_class &clazz);
void print_ptr_decl(ostream &os, const isl_class &clazz);
void print_methods_decl(ostream &os, const isl_class &clazz);
void print_custom_public_decl(ostream &os, const isl_class &clazz);
void print_method_group_decl(ostream &os, const isl_class &clazz,
const string &fullname, const set<FunctionDecl *> &methods);
void print_method_decl(ostream &os, const isl_class &clazz,
Expand All @@ -51,6 +52,7 @@ class cpp_generator : public generator {
void print_destructor_impl(ostream &os, const isl_class &clazz);
void print_ptr_impl(ostream &os, const isl_class &clazz);
void print_methods_impl(ostream &os, const isl_class &clazz);
void print_custom_methods_impl(ostream &os, const isl_class &clazz);
void print_method_group_impl(ostream &os, const isl_class &clazz,
const string &fullname, const set<FunctionDecl *> &methods);
void print_method_impl(ostream &os, const isl_class &clazz,
Expand Down
69 changes: 69 additions & 0 deletions interface/isl_test_cpp.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,71 @@ void test_foreach(isl::ctx ctx)
assert(ret2 == isl::stat::error);
}

/* Test that ids are handled correctly and remain unique.
*
* Verify that id names are stored and returned correctly. Check that ids with
* identical names are equal and those with different names are different.
* Check that ids with identical names and different user pointers are
* different. Verify that equality holds across ids constructed from C and C++
* interfaces.
*/
void test_id(isl::ctx ctx)
{
isl::id id_whatever(ctx, std::string("whatever"));
assert(id_whatever.has_name());
assert(std::string("whatever") == id_whatever.get_name());

isl::id id_other(ctx, std::string("whatever"));
assert(id_whatever == id_other);

int fourtytwo = 42;
isl::id id_whatever_42(ctx, std::string("whatever"), &fourtytwo);
assert(id_whatever != id_whatever_42);

isl::id id_whatever_42_copy(id_whatever_42);
assert(id_whatever_42 == id_whatever_42_copy);

isl::id id_whatever_42_other(ctx, std::string("whatever"), &fourtytwo);
assert(id_whatever_42 == id_whatever_42_other);

isl_id *cid = isl_id_alloc(ctx.get(), "whatever", &fourtytwo);
assert(cid == id_whatever_42.get());
isl_id_free(cid);
}

static void reset_flag(void *user) {
*static_cast<int *>(user) = 0;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add this as a lambda function into the test function below?


/* Test that user pointers of the ids are not freed as long as the exists at
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

theRE exists

* least one id pointing to them, either in C or C++.
*
* In a scope, create an id with a flag as a user object and a user_free that
* resets the flag. Use the id in a set object that lives outside the given
* scope. Check that flag is still set after the id object went out of the
* scope. Check that flag is reset after the set object went of of scope.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

went OUT of scope

*/
void test_id_lifetime(isl::ctx ctx)
{
int *flag = new int(1);

{
isl::set set(ctx, "{:}");
{
isl::id id(ctx, std::string("whatever"), flag,
&reset_flag);
set = set.set_tuple_id(id);
}
assert(1 == *flag);
assert(set.has_tuple_id());

isl::id same_id(ctx, std::string("whatever"), flag);
assert(set.get_tuple_id() == same_id);
}
assert(0 == *flag);
delete flag;
}

/* Test the isl C++ interface
*
* This includes:
Expand All @@ -322,6 +387,8 @@ void test_foreach(isl::ctx ctx)
* - Different parameter types
* - Different return types
* - Foreach functions
* - isl::id uniqueness
* - isl::id lifetime
*/
int main()
{
Expand All @@ -332,6 +399,8 @@ int main()
test_parameters(ctx);
test_return(ctx);
test_foreach(ctx);
test_id(ctx);
test_id_lifetime(ctx);

isl_ctx_free(ctx);
}