[PATCH] D41692: [Polly][WIP] Remove immediate dominator heuristic for error block detection.

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur created this revision.
Meinersbur added reviewers: grosser, jdoerfert.
Meinersbur added a project: Polly.
Herald added a reviewer: bollu.

The heuristic forces blocks that directly follows a loop header to not to be considered error blocks. It was introduced in r249611 with the following commit message:

>   This replaces the support for user defined error functions by a
>   heuristic that tries to determine if a call to a non-pure function
>   should be considered "an error". If so the block is assumed not to be
>   executed at runtime. While treating all non-pure function calls as
>   errors will allow a lot more regions to be analyzed, it will also
>   cause us to dismiss a lot again due to an infeasible runtime context.
>   This patch tries to limit that effect. A non-pure function call is
>   considered an error if it is executed only in conditionally with
>   regards to a cheap but simple heuristic.

In the code below `CCK_Abort2()` would be considered as an error block, but not `CCK_Abort1()` due to this heuristic.

  for (int i = 0; i < n; i+=1) {
    if (ErrorCondition1)
      CCK_Abort1(); // No __attribute__((noreturn))
    if (ErrorCondition2)
      CCK_Abort2(); // No __attribute__((noreturn))
  }

This does not seem useful to me. Checking error conditions in the beginning of some work is quite common. It causes a switch default-case to be not considered an error block in SPEC's cactuBSSN. The comment justifying the heuristic mentions a "load", which does not seem to be applicable here. I propose to remove this heuristic.

This patch currently fails the following unrelated test cases:

  Polly :: ScopDetect/mod_ref_read_pointer.ll
  Polly :: ScopInfo/max-loop-depth.ll
  Polly :: ScopInfo/mod_ref_access_pointee_arguments.ll
  Polly :: ScopInfo/mod_ref_read_pointee_arguments.ll
  Polly :: ScopInfo/mod_ref_read_pointer.ll
  Polly :: ScopInfo/mod_ref_read_pointers.ll


Repository:
  rPLO Polly

https://reviews.llvm.org/D41692

Files:
  lib/Support/ScopHelper.cpp


Index: lib/Support/ScopHelper.cpp
===================================================================
--- lib/Support/ScopHelper.cpp
+++ lib/Support/ScopHelper.cpp
@@ -399,25 +399,6 @@
   if (DominatesAllPredecessors)
     return false;
 
-  // FIXME: This is a simple heuristic to determine if the load is executed
-  //        in a conditional. However, we actually would need the control
-  //        condition, i.e., the post dominance frontier. Alternatively we
-  //        could walk up the dominance tree until we find a block that is
-  //        not post dominated by the load and check if it is a conditional
-  //        or a loop header.
-  auto *DTNode = DT.getNode(&BB);
-  if (!DTNode)
-    return false;
-
-  DTNode = DTNode->getIDom();
-
-  if (!DTNode)
-    return false;
-
-  auto *IDomBB = DTNode->getBlock();
-  if (LI.isLoopHeader(IDomBB))
-    return false;
-
   for (Instruction &Inst : BB)
     if (CallInst *CI = dyn_cast<CallInst>(&Inst)) {
       if (isIgnoredIntrinsic(CI))


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D41692.128498.patch
Type: text/x-patch
Size: 1004 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180103/bf50d32e/attachment.bin>