[PATCH] D32027: [Polly][DeLICM] Use Known information when comparing Existing.Written and Proposed.Written. NFC.

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added a comment.

In https://reviews.llvm.org/D32027#727734, @grosser wrote:

> Hi Michael,
>
> this looks good to me. Could you possibly still add one test case where both Expected and Proposed map to '{}'.


It is already tested by e.g.

  EXPECT_TRUE(
      checkIsConflicting({"{ Dom[i] }", nullptr, "{}"}, {nullptr, "{}", "{}"}));



> (I also would not call this NFC)

Interesting. How do you define "NFC"?

My idea of adding NFC was that this does not (at least it is not intended) change the output of any Polly pass and therefore won't break any compilation. I don't count additional unit-tests and internal interface changes as functional change.



================
Comment at: lib/Transform/DeLICM.cpp:425
+  return Result;
+}
+
----------------
grosser wrote:
> This could possibly be written simpler as
> 
> UMap = UMap.subtract_range(makeUnknown());
> 
> assuming we introduce a makeUnknown() function.
This would turn a linear-time function into an exponential-worst-time function due to its use of the intLP solver. Also, `isl_map_subtract` unconditionally calls normalization functions such as `isl_basic_map_simplify` and `isl_map_compute_divs`.


================
Comment at: lib/Transform/DeLICM.cpp:788
+                             .intersect(filterKnownValInst(Proposed.Written))
+                             .domain();
+
----------------
grosser wrote:
> If you intersect before filtering, you just need to apply the filter once.
The idea of calling `filterKnownValInst` early is that the unknown spaces do not need to be computed just to throw them away afterwards.


https://reviews.llvm.org/D32027