[TypePromotion] Make TypeSize a class member

[samparker]

Summary

Having TypeSize as a static class variable was causing problems with multi-threading. Several static functions have now been converted into methods of TypePromotion and a few other members of TypePromotion and IRPromoter have been added or removed.

Thanks for debugging this @rnk

[samparker]

llvm/lib/CodeGen/TypePromotion.cpp

917

This change was a part of the original commit to fix some tests when the pass is enabled by default. Figured it would be best to get this committed again before re-enabling.

[rnk]

Looks good as far as removing the global goes.

llvm/lib/CodeGen/TypePromotion.cpp

965–966

Unrelated nit: Why not for (auto &I : BB)? The instruction list is a bit of an implementation detail.

[samparker]

llvm/lib/CodeGen/TypePromotion.cpp

965–966

I think this was because I was concerned about modifying the block while iterating through it, but now I can't see why it can't be removed.

llvm/lib/CodeGen/TypePromotion.cpp

 // - they have "sinks": instructions consuming these narrow values.
 //
 // We collect all instruction connecting sources and sinks in a worklist, so
 // that we can mutate these instruction and perform type promotion when it is
 // legal to do so.
 
 namespace {
 class IRPromoter {
+  LLVMContext &Ctx;
+  IntegerType *OrigTy = nullptr;
+  unsigned PromotedWidth = 0;
+  IntegerType *ExtTy = nullptr;
   SmallPtrSet<Value*, 8> NewInsts;
   SmallPtrSet<Instruction*, 4> InstsToRemove;
   DenseMap<Value*, SmallVector<Type*, 4>> TruncTysMap;
   SmallPtrSet<Value*, 8> Promoted;
-  LLVMContext &Ctx;
-  // The type we promote to: always i32
-  IntegerType *ExtTy = nullptr;
-  // The type of the value that the search began from, either i8 or i16.
-  // This defines the max range of the values that we allow in the promoted
-  // tree.
-  IntegerType *OrigTy = nullptr;
   SetVector<Value*> *Visited;
   SmallPtrSetImpl<Value*> *Sources;
   SmallPtrSetImpl<Instruction*> *Sinks;
   SmallPtrSetImpl<Instruction*> *SafeToPromote;
   SmallPtrSetImpl<Instruction*> *SafeWrap;
 
   void ReplaceAllUsersOfWith(Value *From, Value *To);
   void PrepareWrappingAdds(void);
   void ExtendSources(void);
   void ConvertTruncs(void);
   void PromoteTree(void);
   void TruncateSinks(void);
   void Cleanup(void);
 
 public:
-  IRPromoter(Module *M) : Ctx(M->getContext()) { }
-
+  IRPromoter(LLVMContext &C, IntegerType *Ty, unsigned Width) :
+    Ctx(C), OrigTy(Ty), PromotedWidth(Width) {
+    ExtTy = IntegerType::get(Ctx, PromotedWidth);
+    assert(OrigTy->getPrimitiveSizeInBits() < ExtTy->getPrimitiveSizeInBits()
+           && "Original type not smaller than extended type");
+  }
 
-  void Mutate(Type *OrigTy, unsigned PromotedWidth,
-              SetVector<Value*> &Visited,
+  void Mutate(SetVector<Value*> &Visited,
               SmallPtrSetImpl<Value*> &Sources,
               SmallPtrSetImpl<Instruction*> &Sinks,
               SmallPtrSetImpl<Instruction*> &SafeToPromote,
               SmallPtrSetImpl<Instruction*> &SafeWrap);
 };
 
 class TypePromotion : public FunctionPass {
-  IRPromoter *Promoter = nullptr;
+  unsigned TypeSize = 0;
+  LLVMContext *Ctx = nullptr;
+  unsigned RegisterBitWidth = 0;
   SmallPtrSet<Value*, 16> AllVisited;
   SmallPtrSet<Instruction*, 8> SafeToPromote;
   SmallPtrSet<Instruction*, 4> SafeWrap;
 
+  // Does V have the same size result type as TypeSize.
+  bool EqualTypeSize(Value *V);
+  // Does V have the same size, or narrower, result type as TypeSize.
+  bool LessOrEqualTypeSize(Value *V);
+  // Does V have a result type that is wider than TypeSize.
+  bool GreaterThanTypeSize(Value *V);
+  // Does V have a result type that is narrower than TypeSize.
+  bool LessThanTypeSize(Value *V);
+  // Should V be a leaf in the promote tree?
+  bool isSource(Value *V);
+  // Should V be a root in the promotion tree?
+  bool isSink(Value *V);
+  // Should we change the result type of V? It will result in the users of V
+  // being visited.
+  bool shouldPromote(Value *V);
+  // Is I an add or a sub, which isn't marked as nuw, but where a wrapping
+  // result won't affect the computation?
   bool isSafeWrap(Instruction *I);
+  // Can V have its integer type promoted, or can the type be ignored.
+  bool isSupportedType(Value *V);
+  // Is V an instruction with a supported opcode or another value that we can
+  // handle, such as constants and basic blocks.
   bool isSupportedValue(Value *V);
+  // Is V an instruction thats result can trivially promoted, or has safe
+  // wrapping.
   bool isLegalToPromote(Value *V);
   bool TryToPromote(Value *V, unsigned PromotedWidth);
 
 public:
   static char ID;
-  static unsigned TypeSize;
-  Type *OrigTy = nullptr;
 
   TypePromotion() : FunctionPass(ID) {}
 
   void getAnalysisUsage(AnalysisUsage &AU) const override {
     AU.addRequired<TargetTransformInfoWrapperPass>();
     AU.addRequired<TargetPassConfig>();
   }
 
   StringRef getPassName() const override { return PASS_NAME; }
 
-  bool doInitialization(Module &M) override;
   bool runOnFunction(Function &F) override;
-  bool doFinalization(Module &M) override;
 };
 
 }
 
 static bool GenerateSignBits(Value *V) {
   if (!isa<Instruction>(V))
     return false;
 
   unsigned Opc = cast<Instruction>(V)->getOpcode();
   return Opc == Instruction::AShr || Opc == Instruction::SDiv ||
          Opc == Instruction::SRem || Opc == Instruction::SExt;
 }
 
-static bool EqualTypeSize(Value *V) {
-  return V->getType()->getScalarSizeInBits() == TypePromotion::TypeSize;
+bool TypePromotion::EqualTypeSize(Value *V) {
+  return V->getType()->getScalarSizeInBits() == TypeSize;
 }
 
-static bool LessOrEqualTypeSize(Value *V) {
-  return V->getType()->getScalarSizeInBits() <= TypePromotion::TypeSize;
+bool TypePromotion::LessOrEqualTypeSize(Value *V) {
+  return V->getType()->getScalarSizeInBits() <= TypeSize;
 }
 
-static bool GreaterThanTypeSize(Value *V) {
-  return V->getType()->getScalarSizeInBits() > TypePromotion::TypeSize;
+bool TypePromotion::GreaterThanTypeSize(Value *V) {
+  return V->getType()->getScalarSizeInBits() > TypeSize;
 }
 
-static bool LessThanTypeSize(Value *V) {
-  return V->getType()->getScalarSizeInBits() < TypePromotion::TypeSize;
-}
-
-/// Some instructions can use 8- and 16-bit operands, and we don't need to
-/// promote anything larger. We disallow booleans to make life easier when
-/// dealing with icmps but allow any other integer that is <= 16 bits. Void
-/// types are accepted so we can handle switches.
-static bool isSupportedType(Value *V) {
-  Type *Ty = V->getType();
-
-  // Allow voids and pointers, these won't be promoted.
-  if (Ty->isVoidTy() || Ty->isPointerTy())
-    return true;
-
-  if (auto *Ld = dyn_cast<LoadInst>(V))
-    Ty = cast<PointerType>(Ld->getPointerOperandType())->getElementType();
-
-  if (!isa<IntegerType>(Ty) ||
-      cast<IntegerType>(V->getType())->getBitWidth() == 1)
-    return false;
-
-  return LessOrEqualTypeSize(V);
+bool TypePromotion::LessThanTypeSize(Value *V) {
+  return V->getType()->getScalarSizeInBits() < TypeSize;
 }
 
 /// Return true if the given value is a source in the use-def chain, producing
 /// a narrow 'TypeSize' value. These values will be zext to start the promotion
 /// of the tree to i32. We guarantee that these won't populate the upper bits
 /// of the register. ZExt on the loads will be free, and the same for call
 /// return values because we only accept ones that guarantee a zeroext ret val.
 /// Many arguments will have the zeroext attribute too, so those would be free
 /// too.
-static bool isSource(Value *V) {
+bool TypePromotion::isSource(Value *V) {
   if (!isa<IntegerType>(V->getType()))
     return false;
 
   // TODO Allow zext to be sources.
   if (isa<Argument>(V))
     return true;
   else if (isa<LoadInst>(V))
     return true;
   else if (isa<BitCastInst>(V))
     return true;
   else if (auto *Call = dyn_cast<CallInst>(V))
     return Call->hasRetAttr(Attribute::AttrKind::ZExt);
   else if (auto *Trunc = dyn_cast<TruncInst>(V))
     return EqualTypeSize(Trunc);
   return false;
 }
 
 /// Return true if V will require any promoted values to be truncated for the
 /// the IR to remain valid. We can't mutate the value type of these
 /// instructions.
-static bool isSink(Value *V) {
+bool TypePromotion::isSink(Value *V) {
   // TODO The truncate also isn't actually necessary because we would already
   // proved that the data value is kept within the range of the original data
   // type.
 
   // Sinks are:
   // - points where the value in the register is being observed, such as an
   //   icmp, switch or store.
   // - points where value types have to match, such as calls and returns.
if (Total.getBitWidth() > Max.getBitWidth()) {
     if (Total.ugt(Max.zext(Total.getBitWidth())))
       return false;
   } else if (Max.getBitWidth() > Total.getBitWidth()) {
     if (Total.zext(Max.getBitWidth()).ugt(Max))
       return false;
   } else if (Total.ugt(Max))
     return false;
 
-  LLVM_DEBUG(dbgs() << "IR Promotion: Allowing safe overflow for " << *I << "\n");
+  LLVM_DEBUG(dbgs() << "IR Promotion: Allowing safe overflow for "
+             << *I << "\n");
   SafeWrap.insert(I);
   return true;
 }
 
-static bool shouldPromote(Value *V) {
+bool TypePromotion::shouldPromote(Value *V) {
   if (!isa<IntegerType>(V->getType()) || isSink(V))
     return false;
 
   if (isSource(V))
     return true;
 
   auto *I = dyn_cast<Instruction>(V);
   if (!I)
for (unsigned i = 0, e = I->getNumOperands(); i < e; ++i) {
 
       if (auto *Const = dyn_cast<ConstantInt>(Op)) {
         Constant *NewConst = ConstantExpr::getZExt(Const, ExtTy);
         I->setOperand(i, NewConst);
       } else if (isa<UndefValue>(Op))
         I->setOperand(i, UndefValue::get(ExtTy));
     }
 
-    if (shouldPromote(I)) {
+    // Mutate the result type, unless this is an icmp.
+    if (!isa<ICmpInst>(I)) {
       I->mutateType(ExtTy);
       Promoted.insert(I);
     }
   }
 }
 
 void IRPromoter::TruncateSinks() {
   LLVM_DEBUG(dbgs() << "IR Promotion: Fixing up the sinks:\n");
for (auto V : *Visited) {
     }
   }
 
   for (auto *I : InstsToRemove) {
     LLVM_DEBUG(dbgs() << "IR Promotion: Removing " << *I << "\n");
     I->dropAllReferences();
     I->eraseFromParent();
   }
-
-  InstsToRemove.clear();
-  NewInsts.clear();
-  TruncTysMap.clear();
-  Promoted.clear();
-  SafeToPromote->clear();
-  SafeWrap->clear();
 }
 
 void IRPromoter::ConvertTruncs() {
   LLVM_DEBUG(dbgs() << "IR Promotion: Converting truncs..\n");
   IRBuilder<> Builder{Ctx};
 
   for (auto *V : *Visited) {
     if (!isa<TruncInst>(V) || Sources->count(V))
for (auto *V : *Visited) {
 
     if (auto *I = dyn_cast<Instruction>(Masked))
       NewInsts.insert(I);
 
     ReplaceAllUsersOfWith(Trunc, Masked);
   }
 }
 
-void IRPromoter::Mutate(Type *OrigTy, unsigned PromotedWidth,
-                        SetVector<Value*> &Visited,
+void IRPromoter::Mutate(SetVector<Value*> &Visited,
                         SmallPtrSetImpl<Value*> &Sources,
                         SmallPtrSetImpl<Instruction*> &Sinks,
                         SmallPtrSetImpl<Instruction*> &SafeToPromote,
                         SmallPtrSetImpl<Instruction*> &SafeWrap) {
   LLVM_DEBUG(dbgs() << "IR Promotion: Promoting use-def chains from "
-             << TypePromotion::TypeSize << " to " << PromotedWidth
-             << "-bits\n");
-
-  assert(isa<IntegerType>(OrigTy) && "expected integer type");
-  this->OrigTy = cast<IntegerType>(OrigTy);
-  ExtTy = IntegerType::get(Ctx, PromotedWidth);
-  assert(OrigTy->getPrimitiveSizeInBits() < ExtTy->getPrimitiveSizeInBits() &&
-         "original type not smaller than extended type");
+             << OrigTy->getBitWidth() << " to " << PromotedWidth << "-bits\n");
 
   this->Visited = &Visited;
   this->Sources = &Sources;
   this->Sinks = &Sinks;
   this->SafeToPromote = &SafeToPromote;
   this->SafeWrap = &SafeWrap;
 
   // Cache original types of the values that will likely need truncating
void IRPromoter::Mutate(SetVector<Value*> &Visited,
 
   // Finally, remove unecessary zexts and truncs, delete old instructions and
   // clear the data structures.
   Cleanup();
 
   LLVM_DEBUG(dbgs() << "IR Promotion: Mutation complete\n");
 }
 
+/// We disallow booleans to make life easier when dealing with icmps but allow
+/// any other integer that fits in a scalar register. Void types are accepted
+/// so we can handle switches.
+bool TypePromotion::isSupportedType(Value *V) {
+  Type *Ty = V->getType();
+
+  // Allow voids and pointers, these won't be promoted.
+  if (Ty->isVoidTy() || Ty->isPointerTy())
+    return true;
+
+  if (!isa<IntegerType>(Ty) ||
+      cast<IntegerType>(Ty)->getBitWidth() == 1 ||
+      cast<IntegerType>(Ty)->getBitWidth() > RegisterBitWidth)
+    return false;
+
+  return LessOrEqualTypeSize(V);
+}
+
 /// We accept most instructions, as well as Arguments and ConstantInsts. We
 /// Disallow casts other than zext and truncs and only allow calls if their
 /// return value is zeroext. We don't allow opcodes that can introduce sign
 /// bits.
 bool TypePromotion::isSupportedValue(Value *V) {
   if (auto *I = dyn_cast<Instruction>(V)) {
     switch (I->getOpcode()) {
     default:
bool TypePromotion::isLegalToPromote(Value *V) {
   if (isPromotedResultSafe(V) || isSafeWrap(I)) {
     SafeToPromote.insert(I);
     return true;
   }
   return false;
 }
 
 bool TypePromotion::TryToPromote(Value *V, unsigned PromotedWidth) {
-  OrigTy = V->getType();
+  Type *OrigTy = V->getType();
   TypeSize = OrigTy->getPrimitiveSizeInBits();
   SafeToPromote.clear();
   SafeWrap.clear();
 
   if (!isSupportedValue(V) || !shouldPromote(V) || !isLegalToPromote(V))
     return false;
 
   LLVM_DEBUG(dbgs() << "IR Promotion: TryToPromote: " << *V << ", from "
if (isSource(V) || shouldPromote(V)) {
       }
     }
   }
 
   LLVM_DEBUG(dbgs() << "IR Promotion: Visited nodes:\n";
              for (auto *I : CurrentVisited)
                I->dump();
              );
+
   unsigned ToPromote = 0;
+  unsigned NonFreeArgs = 0;
+  SmallPtrSet<BasicBlock*, 4> Blocks;
   for (auto *V : CurrentVisited) {
-    if (Sources.count(V))
+    if (auto *I = dyn_cast<Instruction>(V))
+      Blocks.insert(I->getParent());
+
+    if (Sources.count(V)) {
+      if (auto *Arg = dyn_cast<Argument>(V))
+        if (!Arg->hasZExtAttr() && !Arg->hasSExtAttr())
+          ++NonFreeArgs;
       continue;
+    }
+
     if (Sinks.count(cast<Instruction>(V)))
       continue;
      ++ToPromote;
    }
 
+  // DAG optimisations should be able to handle these cases better, especially
+  // for function arguments.
+  if (ToPromote < 2 || (Blocks.size() == 1 && (NonFreeArgs > SafeWrap.size())))
+    return false;
+
   if (ToPromote < 2)
     return false;
 
-  Promoter->Mutate(OrigTy, PromotedWidth, CurrentVisited, Sources, Sinks,
-                   SafeToPromote, SafeWrap);
+  IRPromoter Promoter(*Ctx, cast<IntegerType>(OrigTy), PromotedWidth);
+  Promoter.Mutate(CurrentVisited, Sources, Sinks, SafeToPromote, SafeWrap);
   return true;
 }
 
-bool TypePromotion::doInitialization(Module &M) {
-  Promoter = new IRPromoter(&M);
-  return false;
-}
-
 bool TypePromotion::runOnFunction(Function &F) {
   if (skipFunction(F) || DisablePromotion)
     return false;
 
   LLVM_DEBUG(dbgs() << "IR Promotion: Running on " << F.getName() << "\n");
 
   auto *TPC = getAnalysisIfAvailable<TargetPassConfig>();
   if (!TPC)
     return false;
 
   bool MadeChange = false;
   const DataLayout &DL = F.getParent()->getDataLayout();
   const TargetMachine &TM = TPC->getTM<TargetMachine>();
   const TargetSubtargetInfo *SubtargetInfo = TM.getSubtargetImpl(F);
   const TargetLowering *TLI = SubtargetInfo->getTargetLowering();
   const TargetTransformInfo &TII =
     getAnalysis<TargetTransformInfoWrapperPass>().getTTI(F);
+  RegisterBitWidth = TII.getRegisterBitWidth(false);
+  Ctx = &F.getParent()->getContext();
 
   // Search up from icmps to try to promote their operands.
   for (BasicBlock &BB : F) {
-    auto &Insts = BB.getInstList();
-    for (auto &I : Insts) {
+    for (auto &I : BB) {
       if (AllVisited.count(&I))
         continue;
 
       if (!isa<ICmpInst>(&I))
         continue;
 
       auto *ICmp = cast<ICmpInst>(&I);
       // Skip signed or pointer compares
for (auto &I : BB) {
           if (SrcVT.isSimple() && TLI->isTypeLegal(SrcVT.getSimpleVT()))
             break;
 
           if (TLI->getTypeAction(ICmp->getContext(), SrcVT) !=
               TargetLowering::TypePromoteInteger)
             break;
 
           EVT PromotedVT = TLI->getTypeToTransformTo(ICmp->getContext(), SrcVT);
-          if (TII.getRegisterBitWidth(false) < PromotedVT.getSizeInBits()) {
+          if (RegisterBitWidth < PromotedVT.getSizeInBits()) {
             LLVM_DEBUG(dbgs() << "IR Promotion: Couldn't find target register "
                        << "for promoted type\n");
             break;
           }
 
           MadeChange |= TryToPromote(I, PromotedVT.getSizeInBits());
           break;
         }
       }
     }
     LLVM_DEBUG(if (verifyFunction(F, &dbgs())) {
                 dbgs() << F;
                 report_fatal_error("Broken function after type promotion");
                });
   }
   if (MadeChange)
     LLVM_DEBUG(dbgs() << "After TypePromotion: " << F << "\n");
 
   return MadeChange;
 }
 
-bool TypePromotion::doFinalization(Module &M) {
-  delete Promoter;
-  return false;
-}
-
 INITIALIZE_PASS_BEGIN(TypePromotion, DEBUG_TYPE, PASS_NAME, false, false)
 INITIALIZE_PASS_END(TypePromotion, DEBUG_TYPE, PASS_NAME, false, false)
 
 char TypePromotion::ID = 0;
-unsigned TypePromotion::TypeSize = 0;
 
 FunctionPass *llvm::createTypePromotionPass() {
   return new TypePromotion();
 }