[TTI] Implement getCastInstrCost

[samparker]

Summary

The logic for the cost of cast instructions was handled in getOperationCost and getCastInstrCost just returned 1. Move the cast cost logic into the specific helper function and also use this in getUserCost.

[hfinkel]

Since most transformations will be using TTI, which should then hold all the required information, just call the base getOperationCost without handling trunc and zext differently.

I don't understand what you mean. You're changing the default TTI implementation that most of the targets will use. Do you only want to call back when isTruncateFree/isZExtFree return false? I'd think that we'd want to continue handling the cases when they return true?

[samparker]

Hi Hal,

Thanks for questions. I have to admit that I've been confused by how TTI is implemented, it seems rather convoluted to me. I've removed my previous change and, instead, I've reorganised some of the logic in TargetTransformInfoImplBase which hopefully makes sense. I can see that implementing isTruncateFree in the ARM backend should also solve my problem and I will look into that next.

Thanks,
sam

[hfinkel]

In D37471#862029, @samparker wrote:

Hi Hal,

Thanks for questions. I have to admit that I've been confused by how TTI is implemented, it seems rather convoluted to me. I've removed my previous change and, instead, I've reorganised some of the logic in TargetTransformInfoImplBase which hopefully makes sense.

It doesn't, likely, but as there's no documentation explicitly explaining this, it's not obvious why. TTI has two cost models right now: First is the "user cost" model, which is used by the inliner, loop unroller, etc. which provides some combination of code size and expense. The second is the model (mainly) used by the vectorizers, and it measures reciprocal throughput. getCastInstrCost is part of the latter model and getUserCost/getOperationCost are part of the former.

That doesn't mean that tying the models together in this way doesn't make sense, it might, but we'd need to be explicit about what we're doing on why. If we don't want to do that here, you should keep the implementations separate.

What problem are you trying to solve?

I can see that implementing isTruncateFree in the ARM backend should also solve my problem and I will look into that next.

Thanks,
sam

[samparker]

Is there any way of telling which API each function is apart of? I was thinking that there was a model driven by TLI and DataLayout and then one specified by a TTI implementation in the backend. Most of the functions end up returning a TargetCostConstant, which I see are the mixed costs you described. Does it make sense to try to bundle all those costs into one enum?

The issue I was having was that my test case wasn't being simplified into a single block because of the estimated cost of the trunc i64. Like I said, I know this is more easily handled by implementing another hook in the backend, but thought it would be better to try something in the mid-end too.

thanks,
sam

[hfinkel]

In D37471#862124, @samparker wrote:

Is there any way of telling which API each function is apart of? I was thinking that there was a model driven by TLI and DataLayout and then one specified by a TTI implementation in the backend. Most of the functions end up returning a TargetCostConstant, which I see are the mixed costs you described. Does it make sense to try to bundle all those costs into one enum?

Both cost models can be overridden by the targets, although there's been, at the present time, more explicit target customization of the reciprocal throughput model used by the vectorizers. TargetCostConstants are currently used by the user-cost model. There isn't really a mid-end-native model (I'm not sure what that would mean, TLI and DL don't really have cost information aside from DL's idea of the maximum size of legal types). Regarding bundling them, so you mean like this: https://reviews.llvm.org/D37170 (Carrot is working on this)?

The issue I was having was that my test case wasn't being simplified into a single block because of the estimated cost of the trunc i64. Like I said, I know this is more easily handled by implementing another hook in the backend, but thought it would be better to try something in the mid-end too.

So you're trying to affect the user-cost model, and implementing the right backend hook seems like the right approach (either by implementing something like isTruncateFree/isZExtFree, or by directly customizing the model if there's more context dependence needed).

thanks,
sam

[samparker]

Ah, nice, yes separating the different costs sounds great. Ok, I will abandon this and implement the target hook. Thanks for your help.

include/llvm/Analysis/TargetTransformInfoImpl.h

     switch (Opcode) {
     default:
       // By default, just classify everything as 'basic'.
       return TTI::TCC_Basic;
 
     case Instruction::GetElementPtr:
       llvm_unreachable("Use getGEPCost for GEP operations!");
 
-    case Instruction::BitCast:
-      assert(OpTy && "Cast instructions must provide the operand type");
-      if (Ty == OpTy || (Ty->isPointerTy() && OpTy->isPointerTy()))
-        // Identity and pointer-to-pointer casts are free.
-        return TTI::TCC_Free;
-
-      // Otherwise, the default basic cost is used.
-      return TTI::TCC_Basic;
-
     case Instruction::FDiv:
     case Instruction::FRem:
     case Instruction::SDiv:
     case Instruction::SRem:
     case Instruction::UDiv:
     case Instruction::URem:
       return TTI::TCC_Expensive;
 
-    case Instruction::IntToPtr: {
-      // An inttoptr cast is free so long as the input is a legal integer type
-      // which doesn't contain values outside the range of a pointer.
-      unsigned OpSize = OpTy->getScalarSizeInBits();
-      if (DL.isLegalInteger(OpSize) &&
-          OpSize <= DL.getPointerTypeSizeInBits(Ty))
-        return TTI::TCC_Free;
-
-      // Otherwise it's not a no-op.
-      return TTI::TCC_Basic;
-    }
-    case Instruction::PtrToInt: {
-      // A ptrtoint cast is free so long as the result is large enough to store
-      // the pointer, and a legal integer type.
-      unsigned DestSize = Ty->getScalarSizeInBits();
-      if (DL.isLegalInteger(DestSize) &&
-          DestSize >= DL.getPointerTypeSizeInBits(OpTy))
-        return TTI::TCC_Free;
-
-      // Otherwise it's not a no-op.
-      return TTI::TCC_Basic;
-    }
+    case Instruction::BitCast:
+    case Instruction::IntToPtr:
+    case Instruction::PtrToInt:
     case Instruction::Trunc:
-      // trunc to a native type is free (assuming the target has compare and
-      // shift-right of the same width).
-      if (DL.isLegalInteger(DL.getTypeSizeInBits(Ty)))
-        return TTI::TCC_Free;
-
-      return TTI::TCC_Basic;
+      return getCastInstrCost(Opcode, Ty, OpTy, nullptr);
     }
   }
 
   int getGEPCost(Type *PointeeType, const Value *Ptr,
                  ArrayRef<const Value *> Operands) {
     // In the basic model, we just assume that all-constant GEPs will be folded
     // into their uses via addressing modes.
     for (unsigned Idx = 0, Size = Operands.size(); Idx != Size; ++Idx)

     return 1;
   }
 
   unsigned getShuffleCost(TTI::ShuffleKind Kind, Type *Ty, int Index,
                           Type *SubTp) {
     return 1;
   }
 
-  unsigned getCastInstrCost(unsigned Opcode, Type *Dst, Type *Src,
-                            const Instruction *I) { return 1; }
+  unsigned getCastInstrCost(unsigned Opcode, Type *Ty, Type *OpTy,
+                            const Instruction *I) {
+    switch(Opcode) {
+    default:
+      return TTI::TCC_Basic;
+    case Instruction::BitCast:
+      assert(OpTy && "Cast instructions must provide the operand type");
+      if (Ty == OpTy || (Ty->isPointerTy() && OpTy->isPointerTy()))
+        // Identity and pointer-to-pointer casts are free.
+        return TTI::TCC_Free;
+
+      // Otherwise, the default basic cost is used.
+      return TTI::TCC_Basic;
+    case Instruction::IntToPtr: {
+      // An inttoptr cast is free so long as the input is a legal integer type
+      // which doesn't contain values outside the range of a pointer.
+      unsigned OpSize = OpTy->getScalarSizeInBits();
+      if (DL.isLegalInteger(OpSize) &&
+          OpSize <= DL.getPointerTypeSizeInBits(Ty))
+        return TTI::TCC_Free;
+
+      // Otherwise it's not a no-op.
+      return TTI::TCC_Basic;
+    }
+    case Instruction::PtrToInt: {
+      // A ptrtoint cast is free so long as the result is large enough to store
+      // the pointer, and a legal integer type.
+      unsigned DestSize = Ty->getScalarSizeInBits();
+      if (DL.isLegalInteger(DestSize) &&
+          DestSize >= DL.getPointerTypeSizeInBits(OpTy))
+        return TTI::TCC_Free;
+
+      // Otherwise it's not a no-op.
+      return TTI::TCC_Basic;
+    }
+    case Instruction::Trunc:
+      // trunc to a native type is free (assuming the target has compare and
+      // shift-right of the same width).
+      if (DL.isLegalInteger(DL.getTypeSizeInBits(Ty)))
+        return TTI::TCC_Free;
+
+      return TTI::TCC_Basic;
+    }
+  }
 
   unsigned getExtractWithExtendCost(unsigned Opcode, Type *Dst,
                                     VectorType *VecTy, unsigned Index) {
     return 1;
   }
 
   unsigned getCFInstrCost(unsigned Opcode) { return 1; }
 

     SmallVector<Type *, 8> ParamTys;
     ParamTys.reserve(Arguments.size());
     for (unsigned Idx = 0, Size = Arguments.size(); Idx != Size; ++Idx)
       ParamTys.push_back(Arguments[Idx]->getType());
     return static_cast<T *>(this)->getIntrinsicCost(IID, RetTy, ParamTys);
   }
 
   unsigned getUserCost(const User *U, ArrayRef<const Value *> Operands) {
+    T* This = static_cast<T*>(this);
     if (isa<PHINode>(U))
       return TTI::TCC_Free; // Model all PHI nodes as free.
 
     if (const GEPOperator *GEP = dyn_cast<GEPOperator>(U)) {
-      return static_cast<T *>(this)->getGEPCost(GEP->getSourceElementType(),
-                                                GEP->getPointerOperand(),
-                                                Operands.drop_front());
+      return This->getGEPCost(GEP->getSourceElementType(),
     }
 
     if (auto CS = ImmutableCallSite(U)) {
       const Function *F = CS.getCalledFunction();
       if (!F) {
         // Just use the called value type.
         Type *FTy = CS.getCalledValue()->getType()->getPointerElementType();
-        return static_cast<T *>(this)
-            ->getCallCost(cast<FunctionType>(FTy), CS.arg_size());
+        return This->getCallCost(cast<FunctionType>(FTy), CS.arg_size());
       }
 
       SmallVector<const Value *, 8> Arguments(CS.arg_begin(), CS.arg_end());
-      return static_cast<T *>(this)->getCallCost(F, Arguments);
+      return This->getCallCost(F, Arguments);
     }
 
     if (const CastInst *CI = dyn_cast<CastInst>(U)) {
       // Result of a cmp instruction is often extended (to be used by other
       // cmp instructions, logical or return instructions). These are usually
       // nop on most sane targets.
       if (isa<CmpInst>(CI->getOperand(0)))
         return TTI::TCC_Free;
       if (isa<SExtInst>(CI) || isa<ZExtInst>(CI) || isa<FPExtInst>(CI))
-        return static_cast<T *>(this)->getExtCost(CI, Operands.back());
+        return This->getExtCost(CI, Operands.back());
+      return This->getCastInstrCost(CI->getOpcode(), U->getType(),
+                                    U->getOperand(0)->getType(), CI);
     }
 
-    return static_cast<T *>(this)->getOperationCost(
-        Operator::getOpcode(U), U->getType(),
+    return This->getOperationCost(Operator::getOpcode(U), U->getType(),
         U->getNumOperands() == 1 ? U->getOperand(0)->getType() : nullptr);
   }
 };
 }
 
 #endif

test/Transforms/SimplifyCFG/ARM/select-trunc-i64.ll

+; RUN: opt -S -simplifycfg -mtriple=arm < %s | FileCheck %s
+target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
+
+; CHECK-LABEL: select_trunc_i64
+; CHECK-NOT: br
+; CHECK: select
+; CHECK: select
+define arm_aapcscc i32 @select_trunc_i64(i32 %a, i32 %b) {
+entry:
+  %conv = sext i32 %a to i64
+  %conv1 = sext i32 %b to i64
+  %add = add nsw i64 %conv1, %conv
+  %cmp = icmp sgt i64 %add, 2147483647
+  br i1 %cmp, label %cond.end7, label %cond.false
+
+cond.false:                                       ; preds = %entry
+  %0 = icmp sgt i64 %add, -2147483648
+  %cond = select i1 %0, i64 %add, i64 -2147483648
+  %extract.t = trunc i64 %cond to i32
+  br label %cond.end7
+
+cond.end7:                                        ; preds = %cond.false, %entry
+  %cond8.off0 = phi i32 [ 2147483647, %entry ], [ %extract.t, %cond.false ]
+  ret i32 %cond8.off0
+}