r373348 - [OPENMP]Fix PR43516: Compiler crash with collapse(2) on non-rectangular

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Tue Oct  1 09:19:10 2019
New Revision: 373348

URL: http://llvm.org/viewvc/llvm-project?rev=373348&view=rev
Log:
[OPENMP]Fix PR43516: Compiler crash with collapse(2) on non-rectangular
loop.

Missed check if the condition is also dependent when building final
expressions for the collapsed loop directives.

Modified:
    cfe/trunk/lib/Sema/SemaOpenMP.cpp
    cfe/trunk/test/OpenMP/for_codegen.cpp

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=373348&r1=373347&r2=373348&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Tue Oct  1 09:19:10 2019
@@ -6247,13 +6247,21 @@ Expr *OpenMPIterationSpaceChecker::build
 Expr *OpenMPIterationSpaceChecker::buildPreCond(
     Scope *S, Expr *Cond,
     llvm::MapVector<const Expr *, DeclRefExpr *> &Captures) const {
+  // Do not build a precondition when the condition/initialization is dependent
+  // to prevent pessimistic early loop exit.
+  // TODO: this can be improved by calculating min/max values but not sure that
+  // it will be very effective.
+  if (CondDependOnLC || InitDependOnLC)
+    return SemaRef.PerformImplicitConversion(
+        SemaRef.ActOnIntegerConstant(SourceLocation(), 1).get(),
+        SemaRef.Context.BoolTy, /*Action=*/Sema::AA_Casting,
+        /*AllowExplicit=*/true).get();
+
   // Try to build LB <op> UB, where <op> is <, >, <=, or >=.
   Sema::TentativeAnalysisScope Trap(SemaRef);
 
-  ExprResult NewLB =
-      InitDependOnLC ? LB : tryBuildCapture(SemaRef, LB, Captures);
-  ExprResult NewUB =
-      CondDependOnLC ? UB : tryBuildCapture(SemaRef, UB, Captures);
+  ExprResult NewLB = tryBuildCapture(SemaRef, LB, Captures);
+  ExprResult NewUB = tryBuildCapture(SemaRef, UB, Captures);
   if (!NewLB.isUsable() || !NewUB.isUsable())
     return nullptr;
 
@@ -7425,7 +7433,7 @@ checkOpenMPLoop(OpenMPDirectiveKind DKin
       Built.DependentCounters[Cnt] = nullptr;
       Built.DependentInits[Cnt] = nullptr;
       Built.FinalsConditions[Cnt] = nullptr;
-      if (IS.IsNonRectangularLB) {
+      if (IS.IsNonRectangularLB || IS.IsNonRectangularUB) {
         Built.DependentCounters[Cnt] =
             Built.Counters[NestedLoopCount - 1 - IS.LoopDependentIdx];
         Built.DependentInits[Cnt] =

Modified: cfe/trunk/test/OpenMP/for_codegen.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/for_codegen.cpp?rev=373348&r1=373347&r2=373348&view=diff
==============================================================================
--- cfe/trunk/test/OpenMP/for_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/for_codegen.cpp Tue Oct  1 09:19:10 2019
@@ -89,23 +89,6 @@ void loop_with_counter_collapse() {
   // CHECK: [[NUM_ITERS_VAL:%.+]] = sub nsw i64 [[MUL]], 1
   // CHECK: store i64 [[NUM_ITERS_VAL]], i64* [[NUM_ITERS:%.+]],
 
-  // Initialization
-  // CHECK: store i32 0, i32* [[I:%.+]],
-  // CHECK: [[I_INIT:%.+]] = load i32, i32* [[I]],
-  // CHECK: store i32 [[I_INIT]], i32* [[J:%.+]],
-
-  // LIFETIME: call void @llvm.lifetime.end
-  // LIFETIME: call void @llvm.lifetime.end
-
-  // Precondition for j counter
-  // CHECK: store i32 0, i32* [[TMP_I:%.+]],
-  // CHECK: [[J_LB_VAL:%.+]] = load i32, i32* [[TMP_I]],
-  // CHECK: [[I_VAL:%.+]] = load i32, i32* [[TMP_I]],
-  // CHECK: [[J_UB_VAL:%.+]] = add nsw i32 4, [[I_VAL]]
-  // CHECK: [[CMP:%.+]] = icmp slt i32 [[J_LB_VAL]], [[J_UB_VAL]]
-  // CHECK: br i1 [[CMP]], label %[[THEN:[^,]+]], label %[[ELSE:[^,]+]]
-
-  // CHECK: [[THEN]]:
   // CHECK: store i64 0, i64* [[LB:%.+]],
   // CHECK: [[NUM_ITERS_VAL:%.+]] = load i64, i64* [[NUM_ITERS]],
   // CHECK: store i64 [[NUM_ITERS_VAL]], i64* [[UB:%.+]],
@@ -633,6 +616,22 @@ void for_with_references() {
     k = cnt;
 }
 
+// CHECK-LABEL: for_with_references_dep_cond
+void for_with_references_dep_cond() {
+// CHECK: [[I:%.+]] = alloca i8,
+// CHECK: [[CNT:%.+]] = alloca i8*,
+// CHECK: [[CNT_PRIV:%.+]] = alloca i8,
+// CHECK: call void @__kmpc_for_static_init_8(
+// CHECK-NOT: load i8, i8* [[CNT]],
+// CHECK: call void @__kmpc_for_static_fini(
+  char i = 0;
+  char &cnt = i;
+#pragma omp for collapse(2)
+  for (cnt = 0; cnt < 2; ++cnt)
+    for (int j = 0; j < 4 + cnt; j++)
+    k = cnt;
+}
+
 struct Bool {
   Bool(bool b) : b(b) {}
   operator bool() const { return b; }