[PATCH] D80604: [MachineCombiner] add a hook to allow some extension for resource length - NFC

[ChenZheng via Phabricator via llvm-commits]

shchenz marked an inline comment as done.
shchenz added a comment.

Thanks for your review. @spatel

On Powerpc, at least on Power9, FMA, FADD, FMUL all have the same latency. so we always prefer FMA to FADD + FMUL in DAGCombiner for now.

Making the combining of FMA in MachineCombiner instead of DAGCombiner like ARM64 in https://reviews.llvm.org/D18751 is not a good idea for PowerPC. As the comment https://reviews.llvm.org/D18751#402906 indicates, there are many patterns to be compared with to get the optimal sequence and MachineCombiner is not designed to get the best pattern. Also extending the current implementation on ARM64 from 1 FMA tuning to a group of FMA's tuning may introduce big complexity.

So now on Powerpc, we decide to make the combining of FMA in DAGCombiner and do specific pattern breaking in Machinecombiner to get good ILP.



================
Comment at: llvm/include/llvm/CodeGen/TargetInstrInfo.h:1099
+  /// The limit on resource length extension we accept in MachineCombiner Pass.
+  virtual int16_t getExtendResourceLenLimit() const { return 0; }
+
----------------
spatel wrote:
> Is there a reason to make this int16_t? If yes, it should be specified in the code comment. If not, just use plain 'int' or 'unsigned'.
No specific reason to use int16_t, I just want to use a small type. int16_t and int should use the same type register as a return value at least on PowerPC target. will be changed to `int` in the commit patch.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80604