From 5cf78607047227d000df187946986c98bd301660 Mon Sep 17 00:00:00 2001 From: Tobias Grosser Date: Thu, 4 Jun 2015 07:44:35 +0000 Subject: [PATCH] Ensure memory access mappings are defined for full domain We now verify that memory access functions imported via JSON are indeed defined for the full iteration domain. Before this change we accidentally imported memory mappings such as i -> i / 127, which only defined a mapped for values of i that are evenly divisible by 127, but which did not define any mapping for the remaining values, with the result that isl just generated an access expression that had undefined behavior for all the unmapped values. In the incorrect test cases, we now either use floor(i/127) or we use p/127 and provide the information that p is indeed a multiple of 127. llvm-svn: 239024 --- polly/lib/Exchange/JSONExporter.cpp | 22 ++++++++++++++++ polly/test/Isl/CodeGen/exprModDiv.ll | 26 +++++++++++-------- .../exprModDiv___%for.cond---%for.end.jscop | 4 +-- ...prModDiv___%for.cond---%for.end.jscop.pow2 | 4 +-- 4 files changed, 41 insertions(+), 15 deletions(-) diff --git a/polly/lib/Exchange/JSONExporter.cpp b/polly/lib/Exchange/JSONExporter.cpp index d8392c38bf0c..d5e66f01562a 100644 --- a/polly/lib/Exchange/JSONExporter.cpp +++ b/polly/lib/Exchange/JSONExporter.cpp @@ -343,6 +343,28 @@ bool JSONImporter::runOnScop(Scop &S) { isl_map_free(newAccessMap); return false; } + + auto NewAccessDomain = isl_map_domain(isl_map_copy(newAccessMap)); + auto CurrentAccessDomain = isl_map_domain(isl_map_copy(currentAccessMap)); + + NewAccessDomain = + isl_set_intersect_params(NewAccessDomain, S.getContext()); + CurrentAccessDomain = + isl_set_intersect_params(CurrentAccessDomain, S.getContext()); + + if (isl_set_is_subset(CurrentAccessDomain, NewAccessDomain) == + isl_bool_false) { + errs() << "Mapping not defined for all iteration domain elements\n"; + isl_set_free(CurrentAccessDomain); + isl_set_free(NewAccessDomain); + isl_map_free(currentAccessMap); + isl_map_free(newAccessMap); + return false; + } + + isl_set_free(CurrentAccessDomain); + isl_set_free(NewAccessDomain); + if (!isl_map_is_equal(newAccessMap, currentAccessMap)) { // Statistics. ++NewAccessMapFound; diff --git a/polly/test/Isl/CodeGen/exprModDiv.ll b/polly/test/Isl/CodeGen/exprModDiv.ll index 81a8f496c254..f3f8b9eb9472 100644 --- a/polly/test/Isl/CodeGen/exprModDiv.ll +++ b/polly/test/Isl/CodeGen/exprModDiv.ll @@ -17,11 +17,15 @@ ; CHECK: %pexp.pdiv_r = urem i64 %polly.indvar, 127 ; CHECK: %polly.access.A6 = getelementptr float, float* %A, i64 %pexp.pdiv_r -; A[i / 127] -; CHECK: %pexp.div = sdiv i64 %polly.indvar, 127 -; CHECK: %polly.access.B8 = getelementptr float, float* %B, i64 %pexp.div +; A[floor(i / 127)] ; -; FIXME: Make isl mark this as an udiv expression. +; Note: without the floor, we would create a map i -> i/127, which only contains +; values of i that are divisible by 127. All other values of i would not +; be mapped to any value. However, to generate correct code we require +; each value of i to indeed be mapped to a value. +; +; CHECK: %pexp.p_div_q = udiv i64 %polly.indvar, 127 +; CHECK: %polly.access.B8 = getelementptr float, float* %B, i64 %pexp.p_div_q ; #define floord(n,d) ((n < 0) ? (n - d + 1) : n) / d ; A[p + 127 * floord(-p - 1, 127) + 127] @@ -38,16 +42,16 @@ ; CHECK: %polly.access.A10 = getelementptr float, float* %A, i64 %24 ; A[p / 127] -; CHECK: %pexp.div12 = sdiv i64 %p, 127 -; CHECK: %polly.access.B13 = getelementptr float, float* %B, i64 %pexp.div12 +; CHECK: %pexp.div = sdiv i64 %p, 127 +; CHECK: %polly.access.B12 = getelementptr float, float* %B, i64 %pexp.div ; A[i % 128] ; POW2: %pexp.pdiv_r = urem i64 %polly.indvar, 128 ; POW2: %polly.access.A6 = getelementptr float, float* %A, i64 %pexp.pdiv_r -; A[i / 128] -; POW2: %pexp.div.shr = ashr i64 %polly.indvar, 7 -; POW2: %polly.access.B8 = getelementptr float, float* %B, i64 %pexp.div +; A[floor(i / 128)] +; POW2: %pexp.p_div_q = udiv i64 %polly.indvar, 128 +; POW2: %polly.access.B8 = getelementptr float, float* %B, i64 %pexp.p_div_q ; #define floord(n,d) ((n < 0) ? (n - d + 1) : n) / d ; A[p + 128 * floord(-p - 1, 128) + 128] @@ -60,8 +64,8 @@ ; POW2: %polly.access.A10 = getelementptr float, float* %A, i64 %24 ; A[p / 128] -; POW2: %pexp.div.shr12 = ashr i64 %p, 7 -; POW2: %polly.access.B13 = getelementptr float, float* %B, i64 %pexp.div.shr12 +; POW2: %pexp.div.shr = ashr i64 %p, 7 +; POW2: %polly.access.B12 = getelementptr float, float* %B, i64 %pexp.div.shr target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" diff --git a/polly/test/Isl/CodeGen/exprModDiv___%for.cond---%for.end.jscop b/polly/test/Isl/CodeGen/exprModDiv___%for.cond---%for.end.jscop index 066c123fa888..807551373990 100644 --- a/polly/test/Isl/CodeGen/exprModDiv___%for.cond---%for.end.jscop +++ b/polly/test/Isl/CodeGen/exprModDiv___%for.cond---%for.end.jscop @@ -1,5 +1,5 @@ { - "context" : "[N, p] -> { : N >= -9223372036854775808 and N <= 9223372036854775807 and p >= -9223372036854775808 and p <= 9223372036854775807 }", + "context" : "[N, p] -> { : N >= -9223372036854775808 and N <= 9223372036854775807 and p >= -9223372036854775808 and p <= 9223372036854775807 and (p % 127) = 0}", "name" : "for.cond => for.end", "statements" : [ { @@ -10,7 +10,7 @@ }, { "kind" : "read", - "relation" : "[N, p] -> { Stmt_for_body[i0] -> MemRef_B[i0 / 127] }" + "relation" : "[N, p] -> { Stmt_for_body[i0] -> MemRef_B[floor(i0 / 127)] }" }, { "kind" : "read", diff --git a/polly/test/Isl/CodeGen/exprModDiv___%for.cond---%for.end.jscop.pow2 b/polly/test/Isl/CodeGen/exprModDiv___%for.cond---%for.end.jscop.pow2 index 5309f8ab5772..f8c14effcce8 100644 --- a/polly/test/Isl/CodeGen/exprModDiv___%for.cond---%for.end.jscop.pow2 +++ b/polly/test/Isl/CodeGen/exprModDiv___%for.cond---%for.end.jscop.pow2 @@ -1,5 +1,5 @@ { - "context" : "[N, p] -> { : N >= -9223372036854775808 and N <= 9223372036854775807 and p >= -9223372036854775808 and p <= 9223372036854775807 }", + "context" : "[N, p] -> { : N >= -9223372036854775808 and N <= 9223372036854775807 and p >= -9223372036854775808 and p <= 9223372036854775807 and p % 128 = 0}", "name" : "for.cond => for.end", "statements" : [ { @@ -10,7 +10,7 @@ }, { "kind" : "read", - "relation" : "[N, p] -> { Stmt_for_body[i0] -> MemRef_B[i0 / 128] }" + "relation" : "[N, p] -> { Stmt_for_body[i0] -> MemRef_B[floor(i0 / 128)] }" }, { "kind" : "read",