[PATCH] D27695: Add Instruction number to LSR cost model (PR23384)

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet requested changes to this revision.
qcolombet added a comment.
This revision now requires changes to proceed.

Hi Evgeny,

Like Wei, I'd like #3 to be a separate patch.
Indeed, #1 and #2 should be NFC as long as the targets do not change the cost model.

Cheers,
-Quentin



================
Comment at: lib/Target/X86/X86TargetTransformInfo.cpp:1874
+                    C2.NumIVMuls, C2.NumBaseAdds,
+                    C2.ScaleCost, C2.ImmCost, C2.SetupCost);
+}
----------------
Do you have data to support that heuristic?
Like Wei said, I suspect this may lead to pretty bad side effect where we will increase the register pressure by a lot to save a few instruction.
So before we switch the default, I want supportive evidence that this is general goodness.

For the record, we discuss with Wei this cost model issue and I still have on my todolist a better register pressure estimation for the loop. E.g., we can ignore NumRegs as long as it is below the regpressure.


Repository:
  rL LLVM

https://reviews.llvm.org/D27695