[PATCH] D147800: [SystemZ] Enable MachineCombiner for FP reassociation.

Jonas Paulsson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Jun 3 05:30:56 PDT 2023


jonpa updated this revision to Diff 528098.
jonpa added a comment.

The Add/Sub/Multiply reassociation seems to work well, so this is adding a handling of FMAs on top of that. This is still under development as it is not yet clear exactly what is the best approach.

- SystemZReassocAdditions.cpp: New experimental optimization that finds chains of FMAs and Adds and reorders computations to minimize stalls. This should help MachineCombiner reassociation but could also perhaps be good on its own. A little unsure of how aggressive this pass should/could be. Right now assuming that if the FmContract flag is passed in addition to FmReassoc and FmNsz, it is ok to take apart FMAs/Adds and form new a new FMA on the top of the chain.

- Reg/mem handling for WFMADB and WFMASB the same way as for the binary instructions.

- Experimental FMA patterns added with tests. Two of them are inspired by the PPC ILP patterns. Have not yet tried these on benchmarks so still mostly unclear what will be the best in the end. One issue might be with longer FMA chains where it might be good to have MachineCombiner step back and revisit the resulting chain of Adds. If it could do that, using FMA2 or FM4 should be the same on a chain of 4 FMAs, for example.

The current idea is to let MachineCombiner decide if a transformation is beneficial, looking at the actual depths of the MachineInstrs using the SchedModel. For the binops, TargetInstrInfo reassociation tries two variants to let MachineCombiner decide which one is improving the Critical Path. For FMA patterns, this is not done so it is kind of arbitrary if a pattern is beneficial - it depends on the incoming operand latencies of the multiplication factors. Maybe that could be done on smaller patterns, like having three or four variants of it, or perhaps the target hook could use the BlockTrace to decide that directly instead. Compile time has been mentioned in comments, so perhaps that would be a good idea (if useful). Then again, the ReassocAdditions pass would be the alternative to doing this, if that turns out to be of good use.


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

https://reviews.llvm.org/D147800

Files:
  llvm/include/llvm/CodeGen/MachineCombinerPattern.h
  llvm/include/llvm/CodeGen/TargetInstrInfo.h
  llvm/lib/CodeGen/MachineCombiner.cpp
  llvm/lib/Target/SystemZ/CMakeLists.txt
  llvm/lib/Target/SystemZ/SystemZ.h
  llvm/lib/Target/SystemZ/SystemZFinalizeReassociation.cpp
  llvm/lib/Target/SystemZ/SystemZISelDAGToDAG.cpp
  llvm/lib/Target/SystemZ/SystemZInstrFP.td
  llvm/lib/Target/SystemZ/SystemZInstrFormats.td
  llvm/lib/Target/SystemZ/SystemZInstrInfo.cpp
  llvm/lib/Target/SystemZ/SystemZInstrInfo.h
  llvm/lib/Target/SystemZ/SystemZInstrVector.td
  llvm/lib/Target/SystemZ/SystemZOperators.td
  llvm/lib/Target/SystemZ/SystemZReassocAdditions.cpp
  llvm/lib/Target/SystemZ/SystemZScheduleZ13.td
  llvm/lib/Target/SystemZ/SystemZScheduleZ14.td
  llvm/lib/Target/SystemZ/SystemZScheduleZ15.td
  llvm/lib/Target/SystemZ/SystemZScheduleZ16.td
  llvm/lib/Target/SystemZ/SystemZTargetMachine.cpp
  llvm/lib/Target/X86/X86InstrInfo.cpp
  llvm/lib/Target/X86/X86InstrInfo.h
  llvm/test/CodeGen/SystemZ/fp-add-02.ll
  llvm/test/CodeGen/SystemZ/fp-mul-02.ll
  llvm/test/CodeGen/SystemZ/machine-combiner-reassoc-fp-01.ll
  llvm/test/CodeGen/SystemZ/machine-combiner-reassoc-fp-02.ll
  llvm/test/CodeGen/SystemZ/machine-combiner-reassoc-fp-03.ll
  llvm/test/CodeGen/SystemZ/machine-combiner-reassoc-fp-04.ll
  llvm/test/CodeGen/SystemZ/machine-combiner-reassoc-fp-05.ll
  llvm/test/CodeGen/SystemZ/machine-combiner-reassoc-fp-06.ll
  llvm/test/CodeGen/SystemZ/machine-combiner-reassoc-fp-07.ll
  llvm/test/CodeGen/SystemZ/machine-combiner-reassoc-fp-08.ll
  llvm/test/CodeGen/SystemZ/machine-combiner-reassoc-fp-09.ll
  llvm/test/CodeGen/SystemZ/reassoc-additions.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D147800.528098.patch
Type: text/x-patch
Size: 193586 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230603/5277ed4e/attachment-0001.bin>


More information about the llvm-commits mailing list