From 4284049dcc3ddf9cf30f110007641596de4b265c Mon Sep 17 00:00:00 2001
From: Florian Hahn <florian.hahn@arm.com>
Date: Mon, 31 Jul 2017 09:00:52 +0000
Subject: [PATCH] [LoopInterchange] Do not interchange loops with function
 calls.

Summary:
Without any information about the called function, we cannot be sure
that it is safe to interchange loops which contain function calls. For
example there could be dependences that prevent interchanging between
accesses in the called function and the loops. Even functions without any
parameters could cause problems, as they could access memory using
global pointers.

For now, I think it is only safe to interchange loops with calls marked
as readnone.

With this patch, the LLVM test suite passes with `-O3 -mllvm
-enable-loopinterchange` and LoopInterchangeProfitability::isProfitable
returning true for all loops. check-llvm and check-clang also pass when
bootstrapped in a similar fashion, although only 3 loops got
interchanged.

Reviewers: karthikthecool, blitz.opensource, hfinkel, mcrosier, mkuper

Reviewed By: mcrosier

Subscribers: mzolotukhin, llvm-commits

Differential Revision: https://reviews.llvm.org/D35489

llvm-svn: 309547
---
 .../lib/Transforms/Scalar/LoopInterchange.cpp |  12 ++
 .../LoopInterchange/call-instructions.ll      | 158 ++++++++++++++++++
 .../interchange-flow-dep-outer.ll             |   4 +-
 .../not-interchanged-tightly-nested.ll        |   2 +-
 4 files changed, 173 insertions(+), 3 deletions(-)
 create mode 100644 llvm/test/Transforms/LoopInterchange/call-instructions.ll

diff --git a/llvm/lib/Transforms/Scalar/LoopInterchange.cpp b/llvm/lib/Transforms/Scalar/LoopInterchange.cpp
index d4ad66fc9df0..2f05ec2b329b 100644
--- a/llvm/lib/Transforms/Scalar/LoopInterchange.cpp
+++ b/llvm/lib/Transforms/Scalar/LoopInterchange.cpp
@@ -958,6 +958,18 @@ bool LoopInterchangeLegality::canInterchangeLoops(unsigned InnerLoopId,
     return false;
   }
 
+  // Check if outer and inner loop contain legal instructions only.
+  for (auto *BB : OuterLoop->blocks())
+    for (Instruction &I : *BB)
+      if (CallInst *CI = dyn_cast<CallInst>(&I)) {
+        // readnone functions do not prevent interchanging.
+        if (CI->doesNotReadMemory())
+          continue;
+        DEBUG(dbgs() << "Loops with call instructions cannot be interchanged "
+                     << "safely.");
+        return false;
+      }
+
   // Create unique Preheaders if we already do not have one.
   BasicBlock *OuterLoopPreHeader = OuterLoop->getLoopPreheader();
   BasicBlock *InnerLoopPreHeader = InnerLoop->getLoopPreheader();
diff --git a/llvm/test/Transforms/LoopInterchange/call-instructions.ll b/llvm/test/Transforms/LoopInterchange/call-instructions.ll
new file mode 100644
index 000000000000..e2428ac53fc2
--- /dev/null
+++ b/llvm/test/Transforms/LoopInterchange/call-instructions.ll
@@ -0,0 +1,158 @@
+; RUN: opt < %s -basicaa -loop-interchange -S | FileCheck %s
+;; We test the complete .ll for adjustment in outer loop header/latch and inner loop header/latch.
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@A = common global [100 x [100 x i32]] zeroinitializer
+
+declare void @foo(i64 %a)
+declare void @bar(i64 %a) readnone
+
+;;--------------------------------------Test case 01------------------------------------
+;; Not safe to interchange, because the called function `foo` is not marked as
+;; readnone, so it could introduce dependences.
+;;
+;;  for(int i=0;i<N;i++) {
+;;    for(int j=1;j<N;j++) {
+;;      foo(i);
+;;      A[j][i] = A[j][i]+k;
+;;    }
+;; }
+
+define void @interchange_01(i32 %k, i32 %N) {
+entry:
+  %cmp21 = icmp sgt i32 %N, 0
+  br i1 %cmp21, label %for1.ph, label %exit
+
+for1.ph:
+  %cmp219 = icmp sgt i32 %N, 1
+  %0 = add i32 %N, -1
+  br label %for1.header
+
+for1.header:
+  %indvars.iv23 = phi i64 [ 0, %for1.ph ], [ %indvars.iv.next24, %for1.inc10 ]
+  br i1 %cmp219, label %for2.ph, label %for1.inc10
+
+for2.ph:
+  br label %for2
+
+for2:
+  %indvars.iv = phi i64 [ %indvars.iv.next, %for2 ], [ 1, %for2.ph ]
+  call void @foo(i64 %indvars.iv23)
+  %arrayidx5 = getelementptr inbounds [100 x [100 x i32]], [100 x [100 x i32]]* @A, i64 0, i64 %indvars.iv, i64 %indvars.iv23
+  %1 = load i32, i32* %arrayidx5
+  %add = add nsw i32 %1, %k
+  store i32 %add, i32* %arrayidx5
+  %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
+  %lftr.wideiv = trunc i64 %indvars.iv to i32
+  %exitcond = icmp eq i32 %lftr.wideiv, %0
+  br i1 %exitcond, label %for2.loopexit , label %for2
+
+for2.loopexit:
+  br label %for1.inc10
+
+for1.inc10:
+  %indvars.iv.next24 = add nuw nsw i64 %indvars.iv23, 1
+  %lftr.wideiv25 = trunc i64 %indvars.iv23 to i32
+  %exitcond26 = icmp eq i32 %lftr.wideiv25, %0
+  br i1 %exitcond26, label %for1.loopexit, label %for1.header
+
+for1.loopexit:
+  br label %exit
+
+exit:
+  ret void
+}
+
+; CHECK-LABEL: @interchange_01
+; CHECK: for1.ph:
+; CHECK: br label %for1.header
+
+; CHECK: for1.header:
+; CHECK-NEXT: %indvars.iv23 = phi i64 [ 0, %for1.ph ], [ %indvars.iv.next24, %for1.inc10 ]
+; CHECK-NEXT: br i1 %cmp219, label %for2.ph, label %for1.inc10
+
+; CHECK: for2:
+; CHECK: br i1 %exitcond, label %for2.loopexit, label %for2
+
+; CHECK: for1.inc10:
+; CHECK: br i1 %exitcond26, label %for1.loopexit, label %for1.header
+
+; CHECK: for1.loopexit:
+; CHECK-NEXT: br label %exit
+
+
+;;--------------------------------------Test case 02------------------------------------
+;; Safe to interchange, because the called function `bar` is marked as readnone,
+;; so it cannot introduce dependences.
+;;
+;;  for(int i=0;i<N;i++) {
+;;    for(int j=1;j<N;j++) {
+;;      bar(i);
+;;      A[j][i] = A[j][i]+k;
+;;    }
+;; }
+
+define void @interchange_02(i32 %k, i32 %N) {
+entry:
+  %cmp21 = icmp sgt i32 %N, 0
+  br i1 %cmp21, label %for1.ph, label %exit
+
+for1.ph:
+  %cmp219 = icmp sgt i32 %N, 1
+  %0 = add i32 %N, -1
+  br label %for1.header
+
+for1.header:
+  %indvars.iv23 = phi i64 [ 0, %for1.ph ], [ %indvars.iv.next24, %for1.inc10 ]
+  br i1 %cmp219, label %for2.ph, label %for1.inc10
+
+for2.ph:
+  br label %for2
+
+for2:
+  %indvars.iv = phi i64 [ %indvars.iv.next, %for2 ], [ 1, %for2.ph ]
+  call void @bar(i64 %indvars.iv23)
+  %arrayidx5 = getelementptr inbounds [100 x [100 x i32]], [100 x [100 x i32]]* @A, i64 0, i64 %indvars.iv, i64 %indvars.iv23
+  %1 = load i32, i32* %arrayidx5
+  %add = add nsw i32 %1, %k
+  store i32 %add, i32* %arrayidx5
+  %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
+  %lftr.wideiv = trunc i64 %indvars.iv to i32
+  %exitcond = icmp eq i32 %lftr.wideiv, %0
+  br i1 %exitcond, label %for2.loopexit , label %for2
+
+for2.loopexit:
+  br label %for1.inc10
+
+for1.inc10:
+  %indvars.iv.next24 = add nuw nsw i64 %indvars.iv23, 1
+  %lftr.wideiv25 = trunc i64 %indvars.iv23 to i32
+  %exitcond26 = icmp eq i32 %lftr.wideiv25, %0
+  br i1 %exitcond26, label %for1.loopexit, label %for1.header
+
+for1.loopexit:
+  br label %exit
+
+exit:
+  ret void
+}
+
+; CHECK-LABEL: @interchange_02
+; CHECK: for1.header:
+; CHECK-NEXT: %indvars.iv23 = phi i64 [ 0, %for1.ph ], [ %indvars.iv.next24, %for1.inc10 ]
+; CHECK-NEXT: br i1 %cmp219, label %for2.split1, label %for1.loopexit
+
+; CHECK: for2.split1:
+; CHECK: br label %for2.loopexit
+
+; CHECK: for2.split:
+; CHECK-NEXT: %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
+; CHECK: br i1 %exitcond, label %for1.loopexit, label %for2
+
+; CHECK: for2.loopexit:
+; CHECK-NEXT:  br label %for1.inc10
+
+; CHECK: for1.inc10:
+; CHECK: br i1 %exitcond26, label %for2.split, label %for1.header
diff --git a/llvm/test/Transforms/LoopInterchange/interchange-flow-dep-outer.ll b/llvm/test/Transforms/LoopInterchange/interchange-flow-dep-outer.ll
index c3b0b9291424..b0b824017482 100644
--- a/llvm/test/Transforms/LoopInterchange/interchange-flow-dep-outer.ll
+++ b/llvm/test/Transforms/LoopInterchange/interchange-flow-dep-outer.ll
@@ -65,8 +65,8 @@ for.body9:                                        ; preds = %for.body9, %for.con
   br i1 %exitcond, label %for.body9, label %for.cond.cleanup8
 }
 
-declare double @fn1()
-declare void @fn2(double)
+declare double @fn1() readnone
+declare void @fn2(double) readnone
 
 
 ;; After interchange %indvars.iv (j) should increment as the middle loop.
diff --git a/llvm/test/Transforms/LoopInterchange/not-interchanged-tightly-nested.ll b/llvm/test/Transforms/LoopInterchange/not-interchanged-tightly-nested.ll
index 0cf91b09e65d..21312b48afe4 100644
--- a/llvm/test/Transforms/LoopInterchange/not-interchanged-tightly-nested.ll
+++ b/llvm/test/Transforms/LoopInterchange/not-interchanged-tightly-nested.ll
@@ -93,7 +93,7 @@ for.end17:
 ; CHECK: ret void
 
 
-declare void @foo(...)
+declare void @foo(...) readnone
 
 ;; Loops not tightly nested are not interchanged
 ;;  for(int j=0;j<N;j++) {