[ARM] Allow truncs as sources in ARM CGP

[samparker]

Summary

We previously only allowed truncs as sinks, but now allow them as sources too. We do this by checking that the result type is the narrow type that we're trying to optimise for.

[SjoerdMeijer]

LGTM

llvm/trunk/lib/Target/ARM/ARMCodeGenPrepare.cpp

return Opc == Instruction::AShr || Opc == Instruction::SDiv ||
          Opc == Instruction::SRem;
 }
 
 /// 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) {
-  LLVM_DEBUG(dbgs() << "ARM CGP: isSupportedType: " << *V << "\n");
   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();
 
   const IntegerType *IntTy = dyn_cast<IntegerType>(Ty);
-  if (!IntTy) {
-    LLVM_DEBUG(dbgs() << "ARM CGP: No, not an integer.\n");
+  if (!IntTy)
     return false;
-  }
 
   return IntTy->getBitWidth() == ARMCodeGenPrepare::TypeSize;
 }
 
 /// Return true if the given value is a source in the use-def chain, producing
 /// a narrow (i8, i16) 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) {
   if (!isa<IntegerType>(V->getType()))
     return false;
-  // TODO Allow truncs and zext to be sources.
+  // 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 isSupportedType(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) {
   // TODO The truncate also isn't actually necessary because we would already
if (Opc == Instruction::Add || Opc == Instruction::Sub) {
     LLVM_DEBUG(dbgs() << "ARM CGP: Allowing safe overflow for " << *I << "\n");
     return true;
   }
 
   return false;
 }
 
 static bool shouldPromote(Value *V) {
-  if (!isa<IntegerType>(V->getType()) || isSink(V)) {
-    LLVM_DEBUG(dbgs() << "ARM CGP: Don't need to promote: " << *V << "\n");
+  if (!isa<IntegerType>(V->getType()) || isSink(V))
     return false;
-  }
 
   if (isSource(V))
     return true;
 
   auto *I = dyn_cast<Instruction>(V);
   if (!I)
     return false;
 
auto ReplaceAllUsersOfWith = [&](Value *From, Value *To) {
     Instruction *InstTo = dyn_cast<Instruction>(To);
     for (Use &U : From->uses()) {
       auto *User = cast<Instruction>(U.getUser());
       if (InstTo && User->isIdenticalTo(InstTo))
         continue;
       Users.push_back(User);
     }
 
-    for (auto &U : Users)
+    for (auto *U : Users)
       U->replaceUsesOfWith(From, To);
   };
 
   auto FixConst = [&](ConstantInt *Const, Instruction *I) {
     Constant *NewConst = isSafeOverflow(I) && Const->isNegative() ?
       ConstantExpr::getSExt(Const, ExtTy) :
       ConstantExpr::getZExt(Const, ExtTy);
     I->replaceUsesOfWith(Const, NewConst);
for (auto I : Sinks) {
     for (unsigned i = 0; i < I->getNumOperands(); ++i) {
       if (Instruction *Trunc = InsertTrunc(I->getOperand(i))) {
         Trunc->moveBefore(I);
         I->setOperand(i, Trunc);
       }
     }
   }
   LLVM_DEBUG(dbgs() << "ARM CGP: Mutation complete:\n");
+  LLVM_DEBUG(dbgs();
+             for (auto *V : Sources)
+               V->dump();
+             for (auto *I : NewInsts)
+               I->dump();
+             for (auto *V : Visited) {
+               if (!Sources.count(V))
+                 V->dump();
+              });
 }
 
 /// 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 ARMCodeGenPrepare::isSupportedValue(Value *V) {
-  LLVM_DEBUG(dbgs() << "ARM CGP: Is " << *V << " supported?\n");
-
   if (isa<ICmpInst>(V))
     return true;
 
   // Memory instructions
   if (isa<StoreInst>(V) || isa<GetElementPtrInst>(V))
     return true;
 
   // Branches and targets.
   if( isa<BranchInst>(V) || isa<SwitchInst>(V) || isa<BasicBlock>(V))
     return true;
 
   // Non-instruction values that we can handle.
   if ((isa<Constant>(V) && !isa<ConstantExpr>(V)) || isa<Argument>(V))
     return isSupportedType(V);
 
   if (isa<PHINode>(V) || isa<SelectInst>(V) || isa<ReturnInst>(V) ||
       isa<LoadInst>(V))
     return isSupportedType(V);
 
-  if (isa<CastInst>(V) && !isa<SExtInst>(V)) 
+  // Truncs can be either sources or sinks.
+  if (auto *Trunc = dyn_cast<TruncInst>(V))
+    return isSupportedType(Trunc) || isSupportedType(Trunc->getOperand(0));
+
+  if (isa<CastInst>(V) && !isa<SExtInst>(V))
     return isSupportedType(cast<CastInst>(V)->getOperand(0));
 
   // Special cases for calls as we need to check for zeroext
   // TODO We should accept calls even if they don't have zeroext, as they can
   // still be sinks.
   if (auto *Call = dyn_cast<CallInst>(V))
     return isSupportedType(Call) &&
            Call->hasRetAttr(Attribute::AttrKind::ZExt);
 
-  if (!isa<BinaryOperator>(V)) {
-    LLVM_DEBUG(dbgs() << "ARM CGP: No, not a binary operator.\n");
+  if (!isa<BinaryOperator>(V))
     return false;
-  }
+
   if (!isSupportedType(V))
     return false;
 
   if (generateSignBits(V)) {
     LLVM_DEBUG(dbgs() << "ARM CGP: No, instruction can generate sign bits.\n");
     return false;
   }
   return true;
while (!WorkList.empty()) {
     // Ignore non-instructions, other than arguments.
     if (!isa<Instruction>(V) && !isSource(V))
       continue;
 
     // If we've already visited this value from somewhere, bail now because
     // the tree has already been explored.
     // TODO: This could limit the transform, ie if we try to promote something
     // from an i8 and fail first, before trying an i16.
-    if (AllVisited.count(V)) {
-      LLVM_DEBUG(dbgs() << "ARM CGP: Already visited this: " << *V << "\n");
+    if (AllVisited.count(V))
       return false;
-    }
 
     CurrentVisited.insert(V);
     AllVisited.insert(V);
 
     // Calls can be both sources and sinks.
     if (isSink(V))
       Sinks.insert(cast<Instruction>(V));
     if (isSource(V))
for (auto &I : Insts) {
 
       if (isa<ICmpInst>(I)) {
         auto &CI = cast<ICmpInst>(I);
 
         // Skip signed or pointer compares
         if (CI.isSigned() || !isa<IntegerType>(CI.getOperand(0)->getType()))
           continue;
 
-        LLVM_DEBUG(dbgs() << "ARM CGP: Searching from: " << CI << "\n");
         for (auto &Op : CI.operands()) {
           if (auto *I = dyn_cast<Instruction>(Op))
             MadeChange |= TryToPromote(I);
         }
       }
     }
     Promoter->Cleanup();
     LLVM_DEBUG(if (verifyFunction(F, &dbgs())) {

llvm/trunk/test/CodeGen/ARM/arm-cgp-casts.ll

 
 exit:
   ret void
 }
 
 ; CHECK-COMMON-LABEL: phi_feeding_switch
 ; CHECK-COMMON: ldrb
 ; CHECK-COMMON: uxtb
-; CHECK-COMMON: uxtb
 define void @phi_feeding_switch(i8* %memblock, i8* %store, i16 %arg) {
 entry:
   %pre = load i8, i8* %memblock, align 1
   %conv = trunc i16 %arg to i8
   br label %header
 
 header:
   %phi.0 = phi i8 [ %pre, %entry ], [ %count, %latch ]