From 30623ffa756d4408b3a04f3f7c2f45ab288fadb1 Mon Sep 17 00:00:00 2001 From: Michael Kruse Date: Tue, 25 Apr 2017 10:57:32 +0000 Subject: [PATCH] [DeLICM] Use Known information when comparing Existing.Occupied and Proposed.Occupied. Do not conflict if the value of Existing and Proposed are the same. This change only affects unit tests, but no functional changes are expected on LLVM-IR, as no Known information is yet extracted and consequently this functionality is only triggered through unit tests. Differential Revision: https://reviews.llvm.org/D32025 git-svn-id: https://llvm.org/svn/llvm-project/polly/trunk@301301 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Transform/DeLICM.cpp | 78 ++++++++++++++++++++++++++++++++++++----- unittests/DeLICM/DeLICMTest.cpp | 23 ++++++++++++ 2 files changed, 93 insertions(+), 8 deletions(-) diff --git a/lib/Transform/DeLICM.cpp b/lib/Transform/DeLICM.cpp index 309dcb7e..b4e57caa 100644 --- a/lib/Transform/DeLICM.cpp +++ b/lib/Transform/DeLICM.cpp @@ -374,6 +374,25 @@ isl::map computeScalarReachingDefinition( // { Domain[] -> Zone[] } return singleton(UMap, ResultSpace); } +/// Create a domain-to-unknown value mapping. +/// +/// Value instances that do not represent a specific value are represented by an +/// unnamed tuple of 0 dimensions. Its meaning depends on the context. It can +/// either mean a specific but unknown value which cannot be represented by +/// other means. It conflicts with itself because those two unknown ValInsts may +/// have different concrete values at runtime. +/// +/// The other meaning is an arbitrary or wildcard value that can be chosen +/// freely, like LLVM's undef. If matched with an unknown ValInst, there is no +/// conflict. +/// +/// @param Domain { Domain[] } +/// +/// @return { Domain[] -> ValInst[] } +isl::union_map makeUnknownForDomain(isl::union_set Domain) { + return give(isl_union_map_from_domain(Domain.take())); +} + /// If InputVal is not defined in the stmt itself, return the MemoryAccess that /// reads the scalar. Return nullptr otherwise (if the value is defined in the /// scop, or is synthesizable). @@ -646,15 +665,58 @@ public: } #endif - // Are the new lifetimes required for Proposed unused in Existing? - if (isl_union_set_is_subset(Proposed.Occupied.keep(), - Existing.Unused.keep()) != isl_bool_true) { + // Do the Existing and Proposed lifetimes conflict? + // + // Lifetimes are described as the cross-product of array elements and zone + // intervals in which they are alive (the space { [Element[] -> Zone[]] }). + // In the following we call this "element/lifetime interval". + // + // In order to not conflict, one of the following conditions must apply for + // each element/lifetime interval: + // + // 1. If occupied in one of the knowledges, it is unused in the other. + // + // - or - + // + // 2. Both contain the same value. + // + // Instead of partitioning the element/lifetime intervals into a part that + // both Knowledges occupy (which requires an expensive subtraction) and for + // these to check whether they are known to be the same value, we check only + // the second condition and ensure that it also applies when then first + // condition is true. This is done by adding a wildcard value to + // Proposed.Known and Existing.Unused such that they match as a common known + // value. We use the "unknown ValInst" for this purpose. Every + // Existing.Unused may match with an unknown Proposed.Occupied because these + // never are in conflict with each other. + auto ProposedOccupiedAnyVal = makeUnknownForDomain(Proposed.Occupied); + auto ProposedValues = Proposed.Known.unite(ProposedOccupiedAnyVal); + + auto ExistingUnusedAnyVal = makeUnknownForDomain(Existing.Unused); + auto ExistingValues = Existing.Known.unite(ExistingUnusedAnyVal); + + auto MatchingVals = ExistingValues.intersect(ProposedValues); + auto Matches = MatchingVals.domain(); + + // Any Proposed.Occupied must either have a match between the known values + // of Existing and Occupied, or be in Existing.Unused. In the latter case, + // the previously added "AnyVal" will match each other. + if (!Proposed.Occupied.is_subset(Matches)) { if (OS) { - auto ConflictingLifetimes = give(isl_union_set_subtract( - Proposed.Occupied.copy(), Existing.Unused.copy())); - OS->indent(Indent) << "Proposed lifetimes are not unused in existing\n"; - OS->indent(Indent) << "Conflicting lifetimes: " << ConflictingLifetimes - << "\n"; + auto Conflicting = Proposed.Occupied.subtract(Matches); + auto ExistingConflictingKnown = + Existing.Known.intersect_domain(Conflicting); + auto ProposedConflictingKnown = + Proposed.Known.intersect_domain(Conflicting); + + OS->indent(Indent) << "Proposed lifetime conflicting with Existing's\n"; + OS->indent(Indent) << "Conflicting occupied: " << Conflicting << "\n"; + if (!ExistingConflictingKnown.is_empty()) + OS->indent(Indent) + << "Existing Known: " << ExistingConflictingKnown << "\n"; + if (!ProposedConflictingKnown.is_empty()) + OS->indent(Indent) + << "Proposed Known: " << ProposedConflictingKnown << "\n"; } return true; } diff --git a/unittests/DeLICM/DeLICMTest.cpp b/unittests/DeLICM/DeLICMTest.cpp index 6fa3f588..cc1e83e5 100644 --- a/unittests/DeLICM/DeLICMTest.cpp +++ b/unittests/DeLICM/DeLICMTest.cpp @@ -241,6 +241,29 @@ TEST(DeLICM, isConflicting) { EXPECT_FALSE(checkIsConflicting({"{ Dom[i] : i != 0 }", nullptr, "{}"}, {"{ Dom[0] }", nullptr, "{}"})); + // Check occupied vs. occupied with known values. + EXPECT_FALSE(checkIsConflictingKnown({"{ Dom[i] -> Val[] }", nullptr, "{}"}, + {"{ Dom[i] -> Val[] }", nullptr, "{}"})); + EXPECT_TRUE(checkIsConflictingKnown({"{ Dom[i] -> ValA[] }", nullptr, "{}"}, + {"{ Dom[i] -> ValB[] }", nullptr, "{}"})); + EXPECT_TRUE(checkIsConflictingKnown({"{ Dom[i] -> Val[] }", nullptr, "{}"}, + {"{ Dom[i] -> [] }", nullptr, "{}"})); + EXPECT_FALSE(checkIsConflictingKnown({"{ Dom[0] -> Val[] }", nullptr, "{}"}, + {nullptr, "{ Dom[0] }", "{}"})); + + // An implementation using subtract would have exponential runtime on patterns + // such as this one. + EXPECT_TRUE(checkIsConflictingKnown( + {"{ Dom[i0,i1,i2,i3,i4,i5,i6,i7,i8,i9,i10,i11,i12,i13,i14,i15]" + "-> Val[] }", + nullptr, "{}"}, + {"[p0,p1,p2,p3,p4,p5,p6,p7,p8,p9,p10,p11,p12,p13,p14,p15,q0," + "q1,q2,q3,q4,q5,q6,q7,q8,q9,q10,q11,q12,q13,q14,q15] -> {" + "Dom[0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0] -> Val[];" + "Dom[p0,p1,p2,p3,p4,p5,p6,p7,p8,p9,p10,p11,p12,p13,p14,p15] -> Val[];" + "Dom[q0,q1,q2,q3,q4,q5,q6,q7,q8,q9,q10,q11,q12,q13,q14,q15] -> Val[] }", + "{}", "{}"})); + // Check occupied vs. written. EXPECT_TRUE( checkIsConflicting({nullptr, "{}", "{}"}, {"{}", nullptr, "{ Dom[0] }"})); -- 2.11.4.GIT