Factored out code that performs IMP caching into a separate class. This can now be used by other passes that cache IMPs outside of loops (for example, always caching class messages because they always have the same receiver).

opts/IMPCacher.cpp

@@ -0,0 +1,76 @@
+#include "llvm/Pass.h"
+#include "llvm/Function.h"
+#include "llvm/Module.h"
+#include "llvm/Instructions.h"
+#include "llvm/GlobalAlias.h"
+#include "llvm/GlobalVariable.h"
+#include "llvm/Constants.h"
+#include "llvm/Support/IRBuilder.h"
+#include "llvm/Analysis/LoopInfo.h"
+#include "llvm/Transforms/Utils/BasicBlockUtils.h"
+#include "IMPCacher.h"
+
+void GNUstep::IMPCacher::CacheLookup(CallInst *lookup, Value *slot, Value
+    *version) {
+
+  BasicBlock *beforeLookupBB = lookup->getParent();
+  BasicBlock *lookupBB = SplitBlock(beforeLookupBB, lookup, Owner);
+  BasicBlock::iterator iter = lookup;
+  iter++;
+  BasicBlock *afterLookupBB = SplitBlock(iter->getParent(), iter, Owner);
+
+  beforeLookupBB->getTerminator()->removeFromParent();
+
+  IRBuilder<> B = IRBuilder<>(beforeLookupBB);
+  // Load the slot and check that neither it nor the version is 0.
+  Value *slotValue = B.CreateLoad(slot);
+  Value *versionValue = B.CreateLoad(version);
+  Value *receiver = lookup->getOperand(1);
+
+  Value *isCacheEmpty = 
+        B.CreateOr(versionValue, B.CreatePtrToInt(slotValue, IntTy));
+  isCacheEmpty = 
+    B.CreateICmpEQ(isCacheEmpty, Constant::getNullValue(IntTy));
+  Value *receiverNotNil =
+     B.CreateICmpNE(receiver, Constant::getNullValue(receiver->getType()));
+  isCacheEmpty = B.CreateAnd(isCacheEmpty, receiverNotNil);
+      
+  BasicBlock *cacheLookupBB = BasicBlock::Create(Context, "cache_check",
+      lookupBB->getParent());
+
+  B.CreateCondBr(isCacheEmpty, lookupBB, cacheLookupBB);
+
+  // Check the cache node is current
+  B.SetInsertPoint(cacheLookupBB);
+  Value *slotVersion = B.CreateStructGEP(slotValue, 3);
+  // Note: Volatile load because the slot version might have changed in
+  // another thread.
+  slotVersion = B.CreateLoad(slotVersion, true, "slot_version");
+  Value *slotCachedFor = B.CreateStructGEP(slotValue, 1);
+  slotCachedFor = B.CreateLoad(slotCachedFor, true, "slot_owner");
+  Value *cls = B.CreateLoad(B.CreateBitCast(receiver, IdTy));
+  Value *isVersionCorrect = B.CreateICmpEQ(slotVersion, versionValue);
+  Value *isOwnerCorrect = B.CreateICmpEQ(slotCachedFor, cls);
+  Value *isSlotValid = B.CreateAnd(isVersionCorrect, isOwnerCorrect);
+  // If this slot is still valid, skip the lookup.
+  B.CreateCondBr(isSlotValid, afterLookupBB, lookupBB);
+
+  // Replace the looked up slot with the loaded one
+  B.SetInsertPoint(afterLookupBB, afterLookupBB->begin());
+  // Not volatile, so a redundant load elimination pass can do some phi
+  // magic with this later.
+  lookup->replaceAllUsesWith(B.CreateLoad(slot));
+
+  // Perform the real lookup and cache the result
+  lookupBB->getTerminator()->removeFromParent();
+  B.SetInsertPoint(lookupBB);
+  // Store it even if the version is 0, because we always check that the
+  // version is not 0 at the start and an occasional redundant store is
+  // probably better than a branch every time.
+  B.CreateStore(lookup, slot);
+  B.CreateStore(B.CreateLoad(B.CreateStructGEP(lookup, 3)), version);
+  cls = B.CreateLoad(B.CreateBitCast(receiver, IdTy));
+  B.CreateStore(cls, B.CreateStructGEP(lookup, 1));
+  B.CreateBr(afterLookupBB);
+}
+

opts/IMPCacher.h

@@ -0,0 +1,31 @@
+namespace llvm
+{
+  class LLVMContext;
+  class Pass;
+  class PointerType;
+  class IntegerType;
+  class CallInst;
+}
+
+using namespace llvm;
+
+namespace GNUstep
+{
+  class IMPCacher
+  {
+    private:
+      LLVMContext &Context;
+      Pass *Owner;
+      const PointerType *PtrTy;
+      const PointerType *IdTy;
+      const IntegerType *IntTy;
+    public:
+      IMPCacher(LLVMContext &C, Pass *owner) : Context(C), Owner(owner) {
+        PtrTy = Type::getInt8PtrTy(Context);
+        // FIXME: 64-bit.
+        IntTy = Type::getInt32Ty(Context);
+        IdTy = PointerType::getUnqual(PtrTy);
+      }
+      void CacheLookup(CallInst *lookup, Value *slot, Value *version);
+  };
+}

opts/LoopIMPCachePass.cpp

@@ -8,6 +8,7 @@
 #include "llvm/Support/IRBuilder.h"
 #include "llvm/Analysis/LoopInfo.h"
 #include "llvm/Transforms/Utils/BasicBlockUtils.h"
+#include "IMPCacher.h"
 #include <string>
 
 using namespace llvm;
@@ -17,104 +18,26 @@ namespace
 {
   class GNULoopIMPCachePass : public FunctionPass 
   {
+    GNUstep::IMPCacher *cacher;
+    const IntegerType *IntTy;
 
     public:
     static char ID;
     GNULoopIMPCachePass() : FunctionPass((intptr_t)&ID) {}
-
-    Module *M;
-    LLVMContext *Context;
-    const PointerType *PtrTy;
-    const PointerType *IdTy;
-    const IntegerType *IntTy;
+    ~GNULoopIMPCachePass() { delete cacher; }
 
     virtual bool doInitialization(Module &Mod) {
-      M = &Mod;
-      Context = &M->getContext();
-      PtrTy = Type::getInt8PtrTy(*Context);
-      // FIXME: 64-bit.
-      IntTy = Type::getInt32Ty(*Context);
-      IdTy = PointerType::getUnqual(PtrTy);
+      cacher = new GNUstep::IMPCacher(Mod.getContext(), this);
+      // FIXME: ILP64
+      IntTy = Type::getInt32Ty(Mod.getContext());
       return false;  
     }
+
     virtual void getAnalysisUsage(AnalysisUsage &Info) const {
       Info.addRequired<LoopInfo>();
     }
 
 
-    // TODO: Move this to a helper class.
-    void CacheLookup(CallInst *lookup, BasicBlock *allocaBlock) {
-      const Type *SlotPtrTy = lookup->getType();
-
-      IRBuilder<> B = IRBuilder<>(allocaBlock);
-      B.SetInsertPoint(allocaBlock, allocaBlock->begin());
-      Value *slot = B.CreateAlloca(SlotPtrTy, 0, "slot");
-      Value *version = B.CreateAlloca(IntTy, 0, "slot_version");
-
-      B.SetInsertPoint(allocaBlock, allocaBlock->getTerminator());
-      B.CreateStore(Constant::getNullValue(SlotPtrTy), slot);
-      B.CreateStore(Constant::getNullValue(IntTy), version);
-
-      BasicBlock *beforeLookupBB = lookup->getParent();
-      BasicBlock *lookupBB = SplitBlock(beforeLookupBB, lookup, this);
-      BasicBlock::iterator iter = lookup;
-      iter++;
-      BasicBlock *afterLookupBB = SplitBlock(iter->getParent(), iter, this);
-
-      beforeLookupBB->getTerminator()->removeFromParent();
-      B.SetInsertPoint(beforeLookupBB);
-      // Load the slot and check that neither it nor the version is 0.
-      Value *slotValue = B.CreateLoad(slot);
-      Value *versionValue = B.CreateLoad(version);
-      Value *receiver = lookup->getOperand(1);
-
-      Value *isCacheEmpty = 
-            B.CreateOr(versionValue, B.CreatePtrToInt(slotValue, IntTy));
-      isCacheEmpty = 
-        B.CreateICmpEQ(isCacheEmpty, Constant::getNullValue(IntTy));
-      Value *receiverNotNil =
-         B.CreateICmpNE(receiver, Constant::getNullValue(receiver->getType()));
-      isCacheEmpty = B.CreateAnd(isCacheEmpty, receiverNotNil);
-          
-      BasicBlock *cacheLookupBB = BasicBlock::Create(*Context, "cache_check",
-          allocaBlock->getParent());
-
-      B.CreateCondBr(isCacheEmpty, lookupBB, cacheLookupBB);
-
-      // Check the cache node is current
-      B.SetInsertPoint(cacheLookupBB);
-      Value *slotVersion = B.CreateStructGEP(slotValue, 3);
-      // Note: Volatile load because the slot version might have changed in
-      // another thread.
-      slotVersion = B.CreateLoad(slotVersion, true, "slot_version");
-      Value *slotCachedFor = B.CreateStructGEP(slotValue, 1);
-      slotCachedFor = B.CreateLoad(slotCachedFor, true, "slot_owner");
-      Value *cls = B.CreateLoad(B.CreateBitCast(receiver, IdTy));
-      Value *isVersionCorrect = B.CreateICmpEQ(slotVersion, versionValue);
-      Value *isOwnerCorrect = B.CreateICmpEQ(slotCachedFor, cls);
-      Value *isSlotValid = B.CreateAnd(isVersionCorrect, isOwnerCorrect);
-      // If this slot is still valid, skip the lookup.
-      B.CreateCondBr(isSlotValid, afterLookupBB, lookupBB);
-
-      // Replace the looked up slot with the loaded one
-      B.SetInsertPoint(afterLookupBB, afterLookupBB->begin());
-      // Not volatile, so a redundant load elimination pass can do some phi
-      // magic with this later.
-      lookup->replaceAllUsesWith(B.CreateLoad(slot));
-
-      // Perform the real lookup and cache the result
-      lookupBB->getTerminator()->removeFromParent();
-      B.SetInsertPoint(lookupBB);
-      // Store it even if the version is 0, because we always check that the
-      // version is not 0 at the start and an occasional redundant store is
-      // probably better than a branch every time.
-      B.CreateStore(lookup, slot);
-      B.CreateStore(B.CreateLoad(B.CreateStructGEP(lookup, 3)), version);
-      cls = B.CreateLoad(B.CreateBitCast(receiver, IdTy));
-      B.CreateStore(cls, B.CreateStructGEP(lookup, 1));
-      B.CreateBr(afterLookupBB);
-    }
-
     virtual bool runOnFunction(Function &F) {
       LoopInfo &LI = getAnalysis<LoopInfo>();
       bool modified = false;
@@ -138,9 +61,18 @@ namespace
           }
         }
       }
+      IRBuilder<> B = IRBuilder<>(entry);
       for (SmallVectorImpl<CallInst*>::iterator i=Lookups.begin(), 
           e=Lookups.end() ; e!=i ; i++) {
-        CacheLookup(*i, entry);
+        const Type *SlotPtrTy = (*i)->getType();
+        B.SetInsertPoint(entry, entry->begin());
+        Value *slot = B.CreateAlloca(SlotPtrTy, 0, "slot");
+        Value *version = B.CreateAlloca(IntTy, 0, "slot_version");
+
+        B.SetInsertPoint(entry, entry->getTerminator());
+        B.CreateStore(Constant::getNullValue(SlotPtrTy), slot);
+        B.CreateStore(Constant::getNullValue(IntTy), version);
+        cacher->CacheLookup(*i, slot, version);
       }
       return modified;
     }