[PATCH] D45534: Add isl operator overloads for isl::pw_aff (Try II)

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur accepted this revision.
Meinersbur added a comment.
This revision is now accepted and ready to land.

In https://reviews.llvm.org/D45534#1064820, @grosser wrote:

> @Meinersbur , your analysis regarding static seems correct. If @philip.pfaffe agrees, I drop the 'static' keyword again.


I don't really care. Compilation might faster with `static` because the compiler does not have to emit those symbols (I'd expect a compiler to not emit the symbols if the function is not used, even with external linkage; because the language mandates that an inline function has a definition in every translation unit, i.e. there is no translation unit that requires the symbol without defining it; it is true for clang, msvc and gcc; I checked. However, only for C++ but not C).

LGTM



================
Comment at: include/polly/Support/ISLOperators.h:110-166
+inline isl::pw_aff operator/(isl::pw_aff Left, isl::pw_aff Right) {
+  return Left.tdiv_q(Right);
+}
+
+inline isl::pw_aff operator/(isl::val ValLeft, isl::pw_aff Right) {
+  isl::pw_aff Left(Right.domain(), ValLeft);
+  return Left.tdiv_q(Right);
----------------
grosser wrote:
> Meinersbur wrote:
> > [comment] Using tdiv as basis for division and remainder corresponds to C/C++'s definition of these operators, i.e. the most consistent choice. 
> > However, in practice this rarely what a user really wants, most often it is modulo (cdiv) or they assume that the divisor is positive or divides the dividend in an integer (divexact). Would not defining a default division/remainder operator an option?
> Very good analysis. We should either add a comment why these operators are choosen or drop them.
> 
> I am open to not having any division / remainder operators defined -- either forever or just as a start. If this is what you want and others agree I can drop this. Is this what you would prefer? Or should I rather add a comment why these have been choosen?r
I'm fine with keeping them. Probably the most typical use cases are with constant-positive divisors: `aff / 2` and `aff % 2`. We probably want to support this use case and it does not matter what the behavior with negative divisors is.


Repository:
  rPLO Polly

https://reviews.llvm.org/D45534