[PATCH] D62907: [ARM] Implement TTI::isHardwareLoopProfitable

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: lib/Target/ARM/ARMTargetTransformInfo.cpp:646
+  default: break;
+  case Intrinsic::sqrt:
+  case Intrinsic::powi:
----------------
There is a floating point sqrt instruction, at least for scalar fp. MVE does not have sqrt or div, but will expand into multiple scalar instructions.


================
Comment at: lib/Target/ARM/ARMTargetTransformInfo.cpp:688
+  case Intrinsic::usub_sat:
+    return true; // FIXME Signed 32-bit could be selected to QADD/SUB.
+  }
----------------
I think most intrinsics like these will be expanded, so not end up as functions (even if they are not very efficient).


================
Comment at: lib/Target/ARM/ARMTargetTransformInfo.cpp:698
+                                          TTI::HardwareLoopInfo &HWLoopInfo) {
+  // FIXME: Low-overhead branches are only supported in the 'low-overhead branch'
+  // extension of v8.1-m.
----------------
This is an extension to 8.1-m? Not just a base feature?


================
Comment at: lib/Target/ARM/ARMTargetTransformInfo.cpp:704
+  // For now, for simplicity, only support loops with one exit block.
+  if (!L->getExitBlock() || !SE.getBackedgeTakenCount(L))
+    return false;
----------------
Will getBackedgeTakenCount ever be nullptr (as opposed to something like SCEVCouldNotCompute below)


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

https://reviews.llvm.org/D62907