[PATCH] D96692: [ARM] Add larger than legal ICmp costs

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.

Looks reasonable.



================
Comment at: llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp:863
+  // will not be needed.
+  const Instruction* Sel = I;
+  if ((Opcode == Instruction::ICmp || Opcode == Instruction::FCmp) && Sel &&
----------------
Nit: ` *Sel`


================
Comment at: llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp:896
+    }
+    if (IID != 0) {
+      // The ICmp is free, the select gets the cost of the min/max/etc
----------------
Nit: `if (IID)`


================
Comment at: llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp:937
+    if (Opcode == Instruction::FCmp && !ST->hasMVEFloatOps()) {
+      return BaseT::getScalarizationOverhead(VecValTy, false, true) +
+             BaseT::getScalarizationOverhead(VecCondTy, true, false) +
----------------
Nit: do we need in comments what these false/true booleans are?


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

https://reviews.llvm.org/D96692