[ARM][ARMCGP] Remove unecessary zexts and truncs

[samparker]

Summary

r345840 slightly changed the way promotion happens which could result in zext and truncs having the same source and destination types. This fixes that issue.

We can now also remove the zext and trunc in the following case:
(zext (trunc (promoted op)), i32)

This means that we can no longer treat a value, that is only used by a sink, to be safe to promote.

I've also added in some extra asserts and replaced a cast for a dyn_cast.

[samparker]

Fixed a bug that allowed zext to be generated with the same source and destination types.

[SjoerdMeijer]

LGTM

lib/Target/ARM/ARMCodeGenPrepare.cpp

694

Nit, instead of:

if (NewInsts.count(Src) && isa<TruncInst>(Src)) {
  auto *Trunc = cast<TruncInst>(Src);
  ...

perhaps this?

auto *Trunc = cast<TruncInst>(Src);
if (NewInsts.count(Src) && Trunc) {
  ...

957

What is this dbgs(); doing? I guess it's not doing much, and a left over from a previous clean-up.

lib/Target/ARM/ARMCodeGenPrepare.cpp

 namespace {
 class IRPromoter {
   SmallPtrSet<Value*, 8> NewInsts;
   SmallVector<Instruction*, 4> InstsToRemove;
   DenseMap<Value*, Type*> TruncTysMap;
   SmallPtrSet<Value*, 8> Promoted;
   Module *M = nullptr;
   LLVMContext &Ctx;
-  Type *ExtTy = nullptr;
-  Type *OrigTy = nullptr;
+  IntegerType *ExtTy = nullptr;
+  IntegerType *OrigTy = nullptr;
 
   void PrepareConstants(SmallPtrSetImpl<Value*> &Visited,
                          SmallPtrSetImpl<Instruction*> &SafeToPromote);
   void ExtendSources(SmallPtrSetImpl<Value*> &Sources);
   void PromoteTree(SmallPtrSetImpl<Value*> &Visited,
                    SmallPtrSetImpl<Value*> &Sources,
                    SmallPtrSetImpl<Instruction*> &Sinks,
                    SmallPtrSetImpl<Instruction*> &SafeToPromote);
   void TruncateSinks(SmallPtrSetImpl<Value*> &Sources,
                      SmallPtrSetImpl<Instruction*> &Sinks);
 
 public:
   IRPromoter(Module *M) : M(M), Ctx(M->getContext()),
                           ExtTy(Type::getInt32Ty(Ctx)) { }
 
   void Cleanup() {
     for (auto *I : InstsToRemove) {
       LLVM_DEBUG(dbgs() << "ARM CGP: Removing " << *I << "\n");
       I->dropAllReferences();
       I->eraseFromParent();
     }
     InstsToRemove.clear();
     NewInsts.clear();
+    TruncTysMap.clear();
+    Promoted.clear();
   }
 
   void Mutate(Type *OrigTy,
               SmallPtrSetImpl<Value*> &Visited,
               SmallPtrSetImpl<Value*> &Sources,
               SmallPtrSetImpl<Instruction*> &Sinks,
               SmallPtrSetImpl<Instruction*> &SafeToPromote);
 };
IRPromoter::PrepareConstants(SmallPtrSetImpl<Value*> &Visited,
   for (auto *I : NewInsts)
     Visited.insert(I);
 }
 
 void IRPromoter::ExtendSources(SmallPtrSetImpl<Value*> &Sources) {
   IRBuilder<> Builder{Ctx};
 
   auto InsertZExt = [&](Value *V, Instruction *InsertPt) {
+    assert(V->getType() != ExtTy && "zext already extends to i32");
     LLVM_DEBUG(dbgs() << "ARM CGP: Inserting ZExt for " << *V << "\n");
     Builder.SetInsertPoint(InsertPt);
     if (auto *I = dyn_cast<Instruction>(V))
       Builder.SetCurrentDebugLocation(I->getDebugLoc());
-    auto *ZExt = cast<Instruction>(Builder.CreateZExt(V, ExtTy));
+
+    Value *ZExt = Builder.CreateZExt(V, ExtTy);
+    if (auto *I = dyn_cast<Instruction>(ZExt)) {
       if (isa<Argument>(V))
-      ZExt->moveBefore(InsertPt);
+        I->moveBefore(InsertPt);
       else
-      ZExt->moveAfter(InsertPt);
+        I->moveAfter(InsertPt);
+      NewInsts.insert(I);
+    }
     ReplaceAllUsersOfWith(V, ZExt);
-    NewInsts.insert(ZExt);
     TruncTysMap[ZExt] = TruncTysMap[V];
   };
 
-
   // Now, insert extending instructions between the sources and their users.
   LLVM_DEBUG(dbgs() << "ARM CGP: Promoting sources:\n");
   for (auto V : Sources) {
     LLVM_DEBUG(dbgs() << " - " << *V << "\n");
     if (auto *I = dyn_cast<Instruction>(V))
       InsertZExt(I, I);
     else if (auto *Arg = dyn_cast<Argument>(V)) {
       BasicBlock &BB = Arg->getParent()->front();

 
 void IRPromoter::Mutate(Type *OrigTy,
                         SmallPtrSetImpl<Value*> &Visited,
                         SmallPtrSetImpl<Value*> &Sources,
                         SmallPtrSetImpl<Instruction*> &Sinks,
                         SmallPtrSetImpl<Instruction*> &SafeToPromote) {
   LLVM_DEBUG(dbgs() << "ARM CGP: Promoting use-def chains to from "
              << ARMCodeGenPrepare::TypeSize << " to 32-bits\n");
-  this->OrigTy = OrigTy;
+
+  assert(isa<IntegerType>(OrigTy) && "expected integer type");
+  this->OrigTy = cast<IntegerType>(OrigTy);
+  assert(OrigTy->getPrimitiveSizeInBits() < ExtTy->getPrimitiveSizeInBits() &&
+         "original type not smaller than extended type");
 
   // Cache original types.
   for (auto *V : Visited)
     TruncTysMap[V] = V->getType();
 
   // Convert adds and subs using negative immediates to equivalent instructions
   // that use positive constants.
   PrepareConstants(Visited, SafeToPromote);
 
   // Insert zext instructions between sources and their users.
   ExtendSources(Sources);
 
   // Promote visited instructions, mutating their types in place. Also insert
   // DSP intrinsics, if enabled, for adds and subs which would be unsafe to
   // promote.
   PromoteTree(Visited, Sources, Sinks, SafeToPromote);
 
for (auto &I : Insts) {
         for (auto &Op : CI.operands()) {
           if (auto *I = dyn_cast<Instruction>(Op))
             MadeChange |= TryToPromote(I);
         }
       }
     }
     Promoter->Cleanup();
     LLVM_DEBUG(if (verifyFunction(F, &dbgs())) {
                 dbgs();
                 report_fatal_error("Broken function after type promotion");
                });
   }
   if (MadeChange)
     LLVM_DEBUG(dbgs() << "After ARMCodeGenPrepare: " << F << "\n");
 
   return MadeChange;
 }