[PATCH] D79164: [CostModel] getCFInstrCost

[Sam Parker via Phabricator via llvm-commits]

samparker marked 2 inline comments as done.
samparker added inline comments.


================
Comment at: llvm/include/llvm/Analysis/TargetTransformInfoImpl.h:433
+    // will require a register.
+    if (Opcode == Instruction::PHI && CostKind != TTI::TCK_RecipThroughput)
+      return 0;
----------------
jsji wrote:
> Looks like this is more aggressive than most target-specific implementation? 
> `CostKind != TTI::TCK_RecipThroughput` is more than  `if (CostKind == TTI::TCK_CodeSize || CostKind == TTI::TCK_SizeAndLatency)`
> 
> Is this intended? 
CodeSize, Latency and SizeAndLatency would generally go through getUserCost first, in which it treats Phis as free, and the backends don't appear to care about Latency. So hopefully this is modelling the previous logic.


================
Comment at: llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp:761
+
+  return 0;
+}
----------------
jsji wrote:
> Any specific reason we would like to force 0 for PowerPC?  
This is because, as this wasn't implemented before, it would have used BasicTTI and returned 0. But I'll check whether I can use the same logic as I've added for X86.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79164/new/

https://reviews.llvm.org/D79164