Simplify construction of parameteric pw affs #25

tobiasgrosser · 2018-05-16T19:58:51Z

This pull request proposes three new functions to simplify the generation of pw_aff expressions on parameter spaces. All expressions are generated on a universe domain.

#include <isl/aff.h>                                                     
__isl_give isl_pw_aff *isl_pw_aff_param_from_id(__isl_take isl_id *id);  
__isl_give isl_pw_aff *isl_pw_aff_val_from_val(__isl_take isl_val *val); 
__isl_give isl_pw_aff *isl_pw_aff_val_from_si(__isl_keep isl_ctx *ctx, int val);

The piplib submodule was removed all the way back isl-0.01-134-g309036cf4f (add copyright statements, Wed Dec 16 19:25:40 2009 +0000), initially by mistake and then properly in isl-0.01-146-gff1ea6be3f (properly remove piplib submodule, Wed Jan 13 09:59:16 2010 +0100). However, it may not get removed completely when moving from an earlier commit to a later one since it may itself leave some untracked files. Just ignore it in that case. Signed-off-by: Sven Verdoolaege <[email protected]>

This function will be reused in the next commit. Signed-off-by: Sven Verdoolaege <[email protected]>

isl_multi_*_from_*_list calls isl_multi_*_set_* on an uninitialized multi expression. If any parameter alignment needs to be performed, then it will fail (without error message) on the uninitialized multi expression while trying to align the missing elements. Perform parameter alignment beforehand. Signed-off-by: Sven Verdoolaege <[email protected]>

This function will be reused in the next commit. Signed-off-by: Sven Verdoolaege <[email protected]>

This makes it more explicit when a multi expression may have missing elements. Signed-off-by: Sven Verdoolaege <[email protected]>

Signed-off-by: Sven Verdoolaege <[email protected]>

…tive The isl AST generator will always produce division of this form. For those created by the user, it's the user's responsibility. Requested-by: Uday Reddy B <[email protected]> Signed-off-by: Sven Verdoolaege <[email protected]>

ftynse

All commit messages have the same typo "piecewiese". Individual comments are mostly typos and minor suggestions.

ftynse · 2018-05-25T13:29:49Z

doc/user.pod

-The following convenience functions first create a base expression and
-then create a piecewise expression over a universe domain.
+The following convenience functions first create a base expression and then
+create a piecewise expression over a universe domain of a given (local) space.


I'm not sure that "over a universe domain of a space" is mathematically correct. "over domain that is a universe set in the given (local) space", leaving that to @skimo-openhub.

ftynse · 2018-05-25T13:31:02Z

doc/user.pod

@@ -3099,6 +3099,14 @@ then create a piecewise expression over a universe domain.
 	__isl_give isl_pw_qpolynomial *isl_pw_qpolynomial_zero(
 		__isl_take isl_space *space);

+The following convenience functions first create a base expression and then
+create a piecewise expression over a universe domain of a parameter space
+which contains exactly one parameter dimension corresponding to the newly


"newly introduced" -> "given" ?

ftynse · 2018-05-25T13:31:14Z

doc/user.pod

+The following convenience functions first create a base expression and then
+create a piecewise expression over a universe domain of a parameter space
+which contains exactly one parameter dimension corresponding to the newly
+introduced parameter dimensions.


"dimension" (singular)

I used 'id' instead.

ftynse · 2018-05-25T13:31:52Z

isl_aff.c

@@ -216,6 +216,26 @@ __isl_give isl_aff *isl_aff_var_on_domain(__isl_take isl_local_space *ls,
 	return NULL;
 }

+/* Return a piecewise affine expression defined over a one dimensional


"one-dimensional"

ftynse · 2018-05-25T13:33:04Z

isl_test.c

+	if (equal < 0)
+		return isl_stat_error;
+	if (!equal)
+		isl_die(ctx, isl_error_unknown, "can not construct pw from id",


Used 'piecewise aff' to be more clear.

ftynse · 2018-05-25T13:33:35Z

isl_test.c

@@ -6048,6 +6075,8 @@ int test_aff(isl_ctx *ctx)
 	isl_aff *aff;
 	int zero, equal;

+	if (test_pwa(ctx) < 0)


I'd call this function test_pa for consistency with the test_upa below

ftynse · 2018-05-25T13:37:18Z

isl_aff.c

@@ -236,6 +236,21 @@ __isl_give isl_pw_aff *isl_pw_aff_param_from_id(__isl_take isl_id *id)
 	return isl_pw_aff_from_aff(aff);
 }

+/* Return a piecewise affine expression defined over a zero dimensional


"zero-dimensional"

ftynse · 2018-05-25T13:38:06Z

isl_test.c

@@ -4765,10 +4765,12 @@ static isl_bool union_pw_aff_plain_is_equal(__isl_keep isl_union_pw_aff *upa,

 /* Basic tests on isl_pw_aff.
 *
- * In particular test the construction of a single-parameter pw_aff from an id.
+ * In particular test the construction of a single-parameter pw_aff from an id


or a zero-parameter pw_aff from an isl val

ftynse · 2018-05-25T13:39:11Z

isl_test.c

+	if (equal < 0)
+		return isl_stat_error;
+	if (!equal)
+		isl_die(ctx, isl_error_unknown, "can not construct pw from val",


from signed integer

This function constructs a new piecewise affine expression defined over a one-dimensional parameter space where the parameter dimension identifier is 'id' and the value of the piecewise affine expression is the single parameter dimension over the full universe domain. This function simplifies the construction of parameter expressions. Specifically, there is no need to talk about set dimensions or even set spaces in case parameter only expressions are constructed. Signed-off-by: Tobias Grosser <[email protected]>

This function constructs a new piecewise affine expression defined over a zero-dimensional parameter space. The value of the piecewise affine expression is the constant value passed to this function. This function simplifies the construction of parameter expressions. Specifically, there is no need to talk about set dimensions or even set spaces in case parameter only expressions are constructed. Constant piecewise expressions with zero parameter dimensions can be combined with other parameter-space only dimensions thanks to isl auto-alignment of parameter dimensions. Signed-off-by: Tobias Grosser <[email protected]>

tobiasgrosser · 2018-05-28T09:36:51Z

@ftynse, thanks for the comments. I addressed them all in the latest version of this patch set.

I missed a high-level comment from you. Do you think the functions themselves and specifically the function names picked make sense?

ftynse · 2018-05-28T09:58:18Z

These functions make sense in context of further set/map construction. I don't see other potential uses now, but may be overlooking something.

I did think about the names. Existing aff construction functions use the pattern "_on_domain", but here we don't have the domain. Alternative names can be along the lines of "isl_pw_aff_cst_on_empty_domain_val", "isl_pw_aff_cst_on_empty_domain_si" and "isl_pw_aff_param_on_singular_domain", but they look too wordy. Using the pattern "<something>_from_<something>" is sort of associated with conversion, like map_from_multi_union_pw_aff, and often exported as overloads or even constructors. For me, a conversion from a constant to a constant function defined over zero-dimensional space sounds okay, not sure about @skimo-openhub.

tobiasgrosser · 2018-05-28T11:01:35Z

OK, so we basically have the following two naming schemes:

Conversion-style naming

These functions transform a constant value / id into a constant function over a zero-dimensional space and are consequently conversion style constructors.

isl_pw_aff_param_from_id
isl_pw_aff_val_from_val
isl_pw_aff_val_from_si

Explicit function names (Alex)

isl_pw_aff_param_on_singular_domain
isl_pw_aff_cst_on_empty_domain_val
isl_pw_aff_cst_on_empty_domain_si

I think we should use param/val, as the term 'constant' might refer both to a parameter or an integer value -- basically everything that does not depend on a variable.

I also don't think that empty_domain is the right name. I would guess, if we call get_domain() on these pw_aff and ask if the domain is_empty() the reply would always be false. Not sure what would be a better name. The term 'singleton' would refer to a set with exactly one element, but does not say anything about the dimensionality of the set. We could use sth like on_param_domain to capture both the fact that the result is a singleton and/or that the space is a parameter space. Now, this won't give information about the number of parameter dimensions. However, as they are anyhow auto-aligned, that might be fine. Maybe the following:

Explicit function names (Tobias)

isl_pw_aff_param_on_param_domain
isl_pw_aff_val_on_param_domain_val
isl_pw_aff_val_on_param_domain_si

tobiasgrosser requested review from Meinersbur and ftynse May 16, 2018 21:01

tobiasgrosser mentioned this pull request May 16, 2018

How to construct isl sets. PollyLabs/isl-interface-documentation#1

Open

Sven Verdoolaege and others added 7 commits May 17, 2018 16:03

isl_*_list_get_*: extract out isl_*_list_peek

e61dd7c

This function will be reused in the next commit. Signed-off-by: Sven Verdoolaege <[email protected]>

isl_multi_*_set_*: extract out isl_multi_*_restore

3624324

This function will be reused in the next commit. Signed-off-by: Sven Verdoolaege <[email protected]>

isl_multi_*_from_*_list: use isl_multi_*_restore

d9063ab

This makes it more explicit when a multi expression may have missing elements. Signed-off-by: Sven Verdoolaege <[email protected]>

Merge branch 'maint'

437e6ab

doc: fix typo in annotation

bb2aed0

Signed-off-by: Sven Verdoolaege <[email protected]>

ftynse reviewed May 25, 2018

View reviewed changes

tobiasgrosser force-pushed the simplify-construction-of-parameteric-pw-affs branch from b7667eb to 0ba9251 Compare May 28, 2018 09:26

tobiasgrosser added 3 commits May 28, 2018 11:32

tobiasgrosser force-pushed the simplify-construction-of-parameteric-pw-affs branch from 0ba9251 to dbfe618 Compare May 28, 2018 09:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simplify construction of parameteric pw affs #25

Simplify construction of parameteric pw affs #25

tobiasgrosser commented May 16, 2018 •

edited

Loading

ftynse left a comment

ftynse May 25, 2018

tobiasgrosser May 28, 2018

ftynse May 25, 2018

tobiasgrosser May 28, 2018

ftynse May 25, 2018

tobiasgrosser May 28, 2018

ftynse May 25, 2018

ftynse May 25, 2018

tobiasgrosser May 28, 2018

ftynse May 25, 2018

tobiasgrosser May 28, 2018

ftynse May 25, 2018

tobiasgrosser May 28, 2018

ftynse May 25, 2018

tobiasgrosser May 28, 2018

ftynse May 25, 2018

tobiasgrosser May 28, 2018

tobiasgrosser commented May 28, 2018

ftynse commented May 28, 2018

tobiasgrosser commented May 28, 2018 •

edited

Loading

Simplify construction of parameteric pw affs #25

Are you sure you want to change the base?

Simplify construction of parameteric pw affs #25

Conversation

tobiasgrosser commented May 16, 2018 • edited Loading

ftynse left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tobiasgrosser commented May 28, 2018

ftynse commented May 28, 2018

tobiasgrosser commented May 28, 2018 • edited Loading

Conversion-style naming

Explicit function names (Alex)

Explicit function names (Tobias)

tobiasgrosser commented May 16, 2018 •

edited

Loading

tobiasgrosser commented May 28, 2018 •

edited

Loading