[PATCH] D42981: [COST] Fix cost model of load instructions on X86

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: lib/Target/X86/X86TargetTransformInfo.cpp:1824
+           !isa<LoadInst>(I->getOperand(1)) || I->getOperand(0) == OpI;
+  case Instruction::Mul:
+    return (!isa<LoadInst>(I->getOperand(0)) ||
----------------
ABataev wrote:
> RKSimon wrote:
> > ABataev wrote:
> > > RKSimon wrote:
> > > > A little tricky, but we only fold certain operands (Operand(1) typically) so if it doesn't commute then we can't fold - ISD::SUB/FSUB/FDIV/SHL/ASHR/LSHR will all definitely be affected by this.
> > > > 
> > > > Also we need to handle the case where both operands are loads - we will still have at least one load cost.
> > > 1. Agree, missed it. Will be fixed.
> > > 2. Yes, it is handled already. The additional checks here are exactly to check this situation.
> > Shift instructions can only fold the second operand as well (and even then very poorly....). I'd be tempted to just not include them in isFreeOp at all tbh.
> No, actually only the first operand can be the memory address and, thus, can be folded
I mean, you're right in terms of asm instructions, but in LLVM IR we shift the first operand, so it must Operand(0), not Operand(1). Also, I limited the size of the data.


Repository:
  rL LLVM

https://reviews.llvm.org/D42981