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

[shchenz]

Summary

Currently preservesResourceLen in machine combiner pass requires that new instructions resource length is not bigger than old instructions resource length.

I think this is ok if size of new instructions is not bigger than size of old instructions. For now I think all reassociations in machine combiner pass meets this condition.

But in patch https://reviews.llvm.org/D80175, we add a new reassociation that combines fma + fma + fma to (fmul + fma + fma + fadd), this will cause callee in getResourceLength, getCycles sometimes return same resource length for new and old instructions and sometimes new resource length is 1 bigger than old resource length.

Add this hook to make it target specific to accept some extension in resource length but we may still benefit from reducing in data depth.

[shchenz]

debug message handling

[spatel]

LGTM with minor fix.

These might also be interesting FMA discussions/examples to look at:
D80801
D18751

llvm/include/llvm/CodeGen/TargetInstrInfo.h

1099

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'.

[shchenz]

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.

llvm/include/llvm/CodeGen/TargetInstrInfo.h

1099

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.

[shchenz]

committed in rG2a24d350dbeacb131af91e8c438fed2bd81698c0

llvm/include/llvm/CodeGen/TargetInstrInfo.h

public:
   /// Attempt to reassociate \P Root and \P Prev according to \P Pattern to
   /// reduce critical path length.
   void reassociateOps(MachineInstr &Root, MachineInstr &Prev,
                       MachineCombinerPattern Pattern,
                       SmallVectorImpl<MachineInstr *> &InsInstrs,
                       SmallVectorImpl<MachineInstr *> &DelInstrs,
                       DenseMap<unsigned, unsigned> &InstrIdxForVirtReg) const;
 
+  /// The limit on resource length extension we accept in MachineCombiner Pass.
+  virtual int16_t getExtendResourceLenLimit() const { return 0; }
+
   /// This is an architecture-specific helper function of reassociateOps.
   /// Set special operand attributes for new instructions after reassociation.
   virtual void setSpecialOperandAttr(MachineInstr &OldMI1, MachineInstr &OldMI2,
                                      MachineInstr &NewMI1,
                                      MachineInstr &NewMI2) const {}
 
   virtual void setSpecialOperandAttr(MachineInstr &MI, uint16_t Flags) const {}
 

llvm/lib/CodeGen/MachineCombiner.cpp

bool MachineCombiner::preservesResourceLen(
   // Compute new resource length.
   unsigned ResLenAfterCombine =
       BlockTrace.getResourceLength(MBBarr, MSCInsArr, MSCDelArr);
 
   LLVM_DEBUG(dbgs() << "\t\tResource length before replacement: "
                     << ResLenBeforeCombine
                     << " and after: " << ResLenAfterCombine << "\n";);
   LLVM_DEBUG(
-      ResLenAfterCombine <= ResLenBeforeCombine
+      ResLenAfterCombine <=
+      ResLenBeforeCombine + TII->getExtendResourceLenLimit()
           ? dbgs() << "\t\t  As result it IMPROVES/PRESERVES Resource Length\n"
           : dbgs() << "\t\t  As result it DOES NOT improve/preserve Resource "
                       "Length\n");
 
-  return ResLenAfterCombine <= ResLenBeforeCombine;
+  return ResLenAfterCombine <=
+         ResLenBeforeCombine + TII->getExtendResourceLenLimit();
 }
 
 /// \returns true when new instruction sequence should be generated
 /// independent if it lengthens critical path or not
 bool MachineCombiner::doSubstitute(unsigned NewSize, unsigned OldSize,
                                    bool OptForSize) {
   if (OptForSize && (NewSize < OldSize))
     return true;