[llvm] r339378 - [LICM] hoist fences out of loops w/o memory operations

[Philip Reames via llvm-commits]

Author: reames
Date: Thu Aug  9 13:18:42 2018
New Revision: 339378

URL: http://llvm.org/viewvc/llvm-project?rev=339378&view=rev
Log:
[LICM] hoist fences out of loops w/o memory operations

The motivating case is an otherwise dead loop with a fence in it. At the moment, this goes all the way through the optimizer and we end up emitting an entirely pointless loop on x86. This case may seem a bit contrived, but we've seen it in real code as the result of otherwise reasonable lowering strategies combined w/thread local memory optimizations (such as escape analysis).

To handle this simple case, we can teach LICM to hoist must execute fences when there is no other memory operation within the loop.

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


Modified:
    llvm/trunk/include/llvm/Analysis/AliasSetTracker.h
    llvm/trunk/lib/Transforms/Scalar/LICM.cpp
    llvm/trunk/test/Transforms/LICM/fence.ll

Modified: llvm/trunk/include/llvm/Analysis/AliasSetTracker.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/AliasSetTracker.h?rev=339378&r1=339377&r2=339378&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Analysis/AliasSetTracker.h (original)
+++ llvm/trunk/include/llvm/Analysis/AliasSetTracker.h Thu Aug  9 13:18:42 2018
@@ -224,6 +224,20 @@ public:
   // track of the list's exact size.
   unsigned size() { return SetSize; }
 
+  /// If this alias set is known to contain a single instruction and *only* a
+  /// single unique instruction, return it.  Otherwise, return nullptr.
+  Instruction* getUniqueInstruction() {
+    if (size() != 0)
+      // Can't track source of pointer, might be many instruction
+      return nullptr;
+    if (AliasAny)
+      // May have collapses alias set
+      return nullptr;
+    if (1 != UnknownInsts.size())
+      return nullptr;
+    return cast<Instruction>(UnknownInsts[0]);
+  }
+
   void print(raw_ostream &OS) const;
   void dump() const;
 

Modified: llvm/trunk/lib/Transforms/Scalar/LICM.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/LICM.cpp?rev=339378&r1=339377&r2=339378&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/LICM.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/LICM.cpp Thu Aug  9 13:18:42 2018
@@ -582,6 +582,7 @@ namespace {
 bool isHoistableAndSinkableInst(Instruction &I) {
   // Only these instructions are hoistable/sinkable.
   return (isa<LoadInst>(I) || isa<CallInst>(I) ||
+          isa<FenceInst>(I) ||
           isa<BinaryOperator>(I) || isa<CastInst>(I) ||
           isa<SelectInst>(I) || isa<GetElementPtrInst>(I) ||
           isa<CmpInst>(I) || isa<InsertElementInst>(I) ||
@@ -684,6 +685,20 @@ bool llvm::canSinkOrHoistInst(Instructio
     // sink the call.
 
     return false;
+  } else if (auto *FI = dyn_cast<FenceInst>(&I)) {
+    // Fences alias (most) everything to provide ordering.  For the moment,
+    // just give up if there are any other memory operations in the loop.
+    auto Begin = CurAST->begin();
+    assert(Begin != CurAST->end() && "must contain FI");
+    if (std::next(Begin) != CurAST->end())
+      // constant memory for instance, TODO: handle better
+      return false;
+    auto *UniqueI = Begin->getUniqueInstruction();
+    if (!UniqueI)
+      // other memory op, give up
+      return false;
+    assert(UniqueI == FI && "AS must contain FI");
+    return true;
   }
 
   assert(!I.mayReadOrWriteMemory() && "unhandled aliasing");

Modified: llvm/trunk/test/Transforms/LICM/fence.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LICM/fence.ll?rev=339378&r1=339377&r2=339378&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/LICM/fence.ll (original)
+++ llvm/trunk/test/Transforms/LICM/fence.ll Thu Aug  9 13:18:42 2018
@@ -3,8 +3,8 @@
 
 define void @test1(i64 %n) {
 ; CHECK-LABEL: @test1
-; CHECK-LABEL: loop:
 ; CHECK: fence
+; CHECK-LABEL: loop:
 entry:
   br label %loop
 loop:
@@ -19,8 +19,8 @@ exit:
 
 define void @test2(i64 %n) {
 ; CHECK-LABEL: @test2
-; CHECK-LABEL: loop:
 ; CHECK: fence
+; CHECK-LABEL: loop:
 entry:
   br label %loop
 loop:
@@ -35,8 +35,8 @@ exit:
 
 define void @test3(i64 %n) {
 ; CHECK-LABEL: @test3
-; CHECK-LABEL: loop:
 ; CHECK: fence
+; CHECK-LABEL: loop:
 entry:
   br label %loop
 loop:
@@ -51,8 +51,8 @@ exit:
 
 define void @test4(i64 %n) {
 ; CHECK-LABEL: @test4
-; CHECK-LABEL: loop:
 ; CHECK: fence
+; CHECK-LABEL: loop:
 entry:
   br label %loop
 loop:
@@ -99,8 +99,10 @@ exit:
   ret void
 }
 
-define void @testfp1(i64 %n, i64* %p) {
-; CHECK-LABEL: @testfp1
+; Note: While a false negative for LICM on it's own, O3 does get this
+; case by combining the fences.
+define void @testfn1(i64 %n, i64* %p) {
+; CHECK-LABEL: @testfn1
 ; CHECK-LABEL: loop:
 ; CHECK: fence
 entry: