[polly] r327216 - Add isl operator overloads for isl::pw_aff
Michael Kruse via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 20 03:50:05 PDT 2018
2018-03-10 19:07 GMT+01:00 Tobias Grosser via llvm-commits
<llvm-commits at lists.llvm.org>:
> Author: grosser
> Date: Sat Mar 10 10:07:03 2018
> New Revision: 327216
>
> URL: http://llvm.org/viewvc/llvm-project?rev=327216&view=rev
> Log:
> Add isl operator overloads for isl::pw_aff
[style] NFC tag missing.
>
> Piecewise affine expressions have directly corresponding mathematical
> operators. Introduce these operators as overloads as this makes writing
> code with isl::pw_aff expressions more directly readable.
>
> We can now write:
>
> A = B + C instead of A = B.add(C)
>
> Added:
> polly/trunk/include/polly/Support/ISLOperators.h
[consistency] Do we use "ISL" or "Isl" in (File)names?
We already have:
IslTest.cpp
ISLTools.{h|cpp}
> Modified:
> polly/trunk/lib/Support/SCEVAffinator.cpp
> polly/trunk/unittests/Isl/IslTest.cpp
>
> Added: polly/trunk/include/polly/Support/ISLOperators.h
> URL: http://llvm.org/viewvc/llvm-project/polly/trunk/include/polly/Support/ISLOperators.h?rev=327216&view=auto
> ==============================================================================
> --- polly/trunk/include/polly/Support/ISLOperators.h (added)
> +++ polly/trunk/include/polly/Support/ISLOperators.h Sat Mar 10 10:07:03 2018
> @@ -0,0 +1,119 @@
> +//===------ IslOperators.h --------------------------------------*- C++ -*-===//
> +//
> +// The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===----------------------------------------------------------------------===//
> +//
> +// Operator overloads for isl C++ objects.
> +//
> +//===----------------------------------------------------------------------===//
> +
[serious] Header guard missing
> +#include "isl/isl-noexceptions.h"
[style] newline after #includes block
> +namespace polly {
> +
> +inline isl::pw_aff operator+(isl::pw_aff A, isl::pw_aff B) { return A.add(B); }
> +
> +inline isl::pw_aff operator+(isl::val V, isl::pw_aff A) {
> + isl::pw_aff AV(A.domain(), V);
> + return A.add(AV);
> +}
> +
> +inline isl::pw_aff operator+(isl::pw_aff A, isl::val V) { return V + A; }
[suggestion] I'd prefer to not use the operators themselves in the
definitions of operator overloads. Unintended infinite recursion can
sneak in easily, e.g. when the other overloads change, or an implicit
conversion is added (long to isl::val), and suddenly a different
overload is called.
> +
> +inline isl::pw_aff operator+(int i, isl::pw_aff A) {
> + isl::ctx ctx = A.get_ctx();
> + return A + isl::val(ctx, i);
> +}
[serious] Use 'long' instead of 'int'. That's what isl::val's ctor takes.
> +
> +inline isl::pw_aff operator+(isl::pw_aff A, int i) { return i + A; }
> +
> +inline isl::pw_aff operator*(isl::pw_aff A, isl::pw_aff B) { return A.mul(B); }
> +
> +inline isl::pw_aff operator*(isl::val V, isl::pw_aff A) {
> + isl::pw_aff AV(A.domain(), V);
> + return A.add(AV);
> +}
> +
> +inline isl::pw_aff operator*(isl::pw_aff A, isl::val V) { return V * A; }
> +
> +inline isl::pw_aff operator*(int i, isl::pw_aff A) {
> + isl::ctx ctx = A.get_ctx();
> + return A * isl::val(ctx, i);
> +}
> +
> +inline isl::pw_aff operator*(isl::pw_aff A, int i) { return i * A; }
> +
> +inline isl::pw_aff operator-(isl::pw_aff A, isl::pw_aff B) { return A.sub(B); }
> +
> +inline isl::pw_aff operator-(isl::val V, isl::pw_aff A) {
> + isl::pw_aff AV(A.domain(), V);
> + return AV - A;
> +}
> +
> +inline isl::pw_aff operator-(isl::pw_aff A, isl::val V) {
> + isl::pw_aff AV(A.domain(), V);
> + return A - AV;
> +}
> +
> +inline isl::pw_aff operator-(int i, isl::pw_aff A) {
> + isl::ctx ctx = A.get_ctx();
> + return isl::val(ctx, i) - A;
> +}
> +
> +inline isl::pw_aff operator-(isl::pw_aff A, int i) {
> + isl::ctx ctx = A.get_ctx();
> + return A - isl::val(ctx, i);
> +}
> +
> +inline isl::pw_aff operator/(isl::pw_aff A, isl::pw_aff B) {
> + return A.tdiv_q(B);
> +}
> +
> +inline isl::pw_aff operator/(isl::val V, isl::pw_aff A) {
> + isl::pw_aff AV(A.domain(), V);
> + return AV / A;
> +}
> +
> +inline isl::pw_aff operator/(isl::pw_aff A, isl::val V) {
> + isl::pw_aff AV(A.domain(), V);
> + return A / AV;
> +}
> +
> +inline isl::pw_aff operator/(int i, isl::pw_aff A) {
> + isl::ctx ctx = A.get_ctx();
> + return isl::val(ctx, i) / A;
> +}
> +
> +inline isl::pw_aff operator/(isl::pw_aff A, int i) {
> + isl::ctx ctx = A.get_ctx();
> + return A / isl::val(ctx, i);
> +}
> +
> +inline isl::pw_aff operator%(isl::pw_aff A, isl::pw_aff B) {
> + return A.tdiv_r(B);
> +}
> +
> +inline isl::pw_aff operator%(isl::val V, isl::pw_aff A) {
> + isl::pw_aff AV(A.domain(), V);
> + return AV % A;
> +}
> +
> +inline isl::pw_aff operator%(isl::pw_aff A, isl::val V) {
> + isl::pw_aff AV(A.domain(), V);
> + return A % AV;
> +}
> +
> +inline isl::pw_aff operator%(int i, isl::pw_aff A) {
> + isl::ctx ctx = A.get_ctx();
> + return isl::val(ctx, i) % A;
> +}
> +
> +inline isl::pw_aff operator%(isl::pw_aff A, int i) {
> + isl::ctx ctx = A.get_ctx();
> + return A % isl::val(ctx, i);
> +}
> +
[suggestion] Also overload !=, ==, +=, -=. *=, /=.
> +} // namespace polly
>
> Modified: polly/trunk/lib/Support/SCEVAffinator.cpp
[suggestion] The commit message only mentions that such overloads are
added, but not that one selected place is also changed to use it.
Would it have been a good idea to separate this into a separate
commit?
> URL: http://llvm.org/viewvc/llvm-project/polly/trunk/lib/Support/SCEVAffinator.cpp?rev=327216&r1=327215&r2=327216&view=diff
> ==============================================================================
> --- polly/trunk/lib/Support/SCEVAffinator.cpp (original)
> +++ polly/trunk/lib/Support/SCEVAffinator.cpp Sat Mar 10 10:07:03 2018
> @@ -15,6 +15,7 @@
> #include "polly/Options.h"
> #include "polly/ScopInfo.h"
> #include "polly/Support/GICHelper.h"
> +#include "polly/Support/ISLOperators.h"
> #include "polly/Support/SCEVValidator.h"
> #include "polly/Support/ScopHelper.h"
> #include "isl/aff.h"
> @@ -162,7 +163,8 @@ isl::pw_aff SCEVAffinator::addModuloSema
> isl::set Domain = PWA.domain();
> isl::pw_aff AddPW =
> isl::manage(getWidthExpValOnDomain(Width - 1, Domain.take()));
> - return PWA.add(AddPW).mod(ModVal).sub(AddPW);
> +
> + return ((PWA + AddPW) % ModVal) - AddPW;
> }
>
> bool SCEVAffinator::hasNSWAddRecForLoop(Loop *L) const {
>
> Modified: polly/trunk/unittests/Isl/IslTest.cpp
> URL: http://llvm.org/viewvc/llvm-project/polly/trunk/unittests/Isl/IslTest.cpp?rev=327216&r1=327215&r2=327216&view=diff
> ==============================================================================
> --- polly/trunk/unittests/Isl/IslTest.cpp (original)
> +++ polly/trunk/unittests/Isl/IslTest.cpp Sat Mar 10 10:07:03 2018
> @@ -8,6 +8,7 @@
> //===----------------------------------------------------------------------===//
>
> #include "polly/Support/GICHelper.h"
> +#include "polly/Support/ISLOperators.h"
> #include "polly/Support/ISLTools.h"
> #include "gtest/gtest.h"
> #include "isl/stream.h"
> @@ -74,6 +75,10 @@ static bool operator==(const isl::union_
> static bool operator==(const isl::val &LHS, const isl::val &RHS) {
> return bool(LHS.eq(RHS));
> }
> +
> +static bool operator==(const isl::pw_aff &LHS, const isl::pw_aff &RHS) {
> + return bool(LHS.is_equal(RHS));
> +}
> } // namespace noexceptions
> } // namespace isl
>
> @@ -279,6 +284,70 @@ TEST(Isl, IslValToAPInt) {
> }
>
> isl_ctx_free(IslCtx);
> +}
> +
> +TEST(Isl, Operators) {
> + isl_ctx *IslCtx = isl_ctx_alloc();
> +
> + isl::val ValOne = isl::val(IslCtx, 1);
> + isl::val ValTwo = isl::val(IslCtx, 2);
> + isl::val ValThree = isl::val(IslCtx, 3);
> + isl::val ValFour = isl::val(IslCtx, 4);
> +
> + isl::space Space = isl::space(IslCtx, 0, 0);
> + isl::local_space LS = isl::local_space(Space);
> +
> + isl::pw_aff AffOne = isl::aff(LS, ValOne);
> + isl::pw_aff AffTwo = isl::aff(LS, ValTwo);
> + isl::pw_aff AffThree = isl::aff(LS, ValThree);
> + isl::pw_aff AffFour = isl::aff(LS, ValFour);
> +
> + // Addition
> + {
> + EXPECT_EQ(AffOne + AffOne, AffTwo);
> + EXPECT_EQ(AffOne + 1, AffTwo);
> + EXPECT_EQ(1 + AffOne, AffTwo);
> + EXPECT_EQ(AffOne + ValOne, AffTwo);
> + EXPECT_EQ(ValOne + AffOne, AffTwo);
> + }
> +
> + // Multiplication
> + {
> + EXPECT_EQ(AffTwo * AffTwo, AffFour);
> + EXPECT_EQ(AffTwo * 2, AffFour);
> + EXPECT_EQ(2 * AffTwo, AffFour);
> + EXPECT_EQ(AffTwo * ValTwo, AffFour);
> + EXPECT_EQ(ValTwo * AffTwo, AffFour);
> + }
> +
> + // Subtraction
> + {
> + EXPECT_EQ(AffTwo - AffOne, AffOne);
> + EXPECT_EQ(AffTwo - 1, AffOne);
> + EXPECT_EQ(2 - AffOne, AffOne);
> + EXPECT_EQ(AffTwo - ValOne, AffOne);
> + EXPECT_EQ(ValTwo - AffOne, AffOne);
> + }
> +
> + // Division
> + {
> + EXPECT_EQ(AffFour - AffTwo, AffTwo);
> + EXPECT_EQ(AffFour - 2, AffTwo);
> + EXPECT_EQ(4 - AffTwo, AffTwo);
> + EXPECT_EQ(AffFour / ValTwo, AffTwo);
> + EXPECT_EQ(AffFour / 2, AffTwo);
> + }
> +
> + // Remainder
> + {
> + EXPECT_EQ(AffThree % AffTwo, AffOne);
> + EXPECT_EQ(AffThree % 2, AffOne);
> + EXPECT_EQ(3 % AffTwo, AffOne);
> + EXPECT_EQ(AffThree % ValTwo, AffOne);
> + EXPECT_EQ(ValThree % AffTwo, AffOne);
> + }
[suggestion] For division and remainder, some cases with negative
values would be helpful. To ensure that the operators correspond to
tdiv (and not, e.g. cdiv of fdiv).
> +
> + isl_ctx_free(IslCtx);
> }
>
> TEST(Isl, Foreach) {
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list