[PATCH] D70913: [FPEnv][SelectionDAG] Relax chain requirements

[Ulrich Weigand via Phabricator via llvm-commits]

uweigand created this revision.
uweigand added reviewers: cameron.mcinally, craig.topper, andrew.w.kaylor, kpn, arsenm.
Herald added subscribers: llvm-commits, hiraditya, wdng.
Herald added a project: LLVM.

This patch fixes one remaining problem in matching strict FP operations in the SystemZ back-end:  it is currently not possible to have the DAG matcher accept certain complex expressions involving strict FP operations; specifically this affects matching the MDEB(R) etc. family of multiply instructions that implicitly extend their input (32x32 -> 64 bit or 64x64 -> 128 bit).

The matcher currently fails because of two reasons:

1. FP intrinsics are too rigidly sequenced due to the way their chains are constructed
2. The chain output confuses a hasOneUse check in the matcher

As to 1), SelectionDAGBuilder::visitConstrainedFPIntrinsic currently treats each constrained intrinsic like a global barrier (e.g. a function call) and fully serializes all pending chains.  This is actually not required; it is allowed for constrained intrinsics to be reordered w.r.t one another or (nonvolatile) memory accesses.  The MI-level scheduler already allows for that flexibility, so it makes sense to allow it at the DAG level as well.

This patch therefore changes the way chains for constrained intrisincs are created, and handles them basically like load operations are handled.  This has the effect that constrained intrinsics are no longer serialized against one another or (nonvolatile) loads.  They are still serialized against stores, but that seems hard to change with the current DAG chain setup, and it also doesn't seem to be a big problem preventing DAG optimizations.

As to 2), even with the above change, I still was unable to match more that two linked DAG nodes in a single pattern (e.g. load -> extend -> multiply).  This is because the OPC_CheckFoldableChainNode check requires that each of the intermediate nodes ("extend" in the above example) only has a single use.  This check fails for strict operations as the chain output of the extend indeed has another use.  However, we don't really need to consider chains here at all, since they will all be rewritten anyway by UpdateChains later.  Other parts of the matcher therefore already ignore chains, but this hasOneUse check doesn't.

This patch replaces hasOneUse by a custom test that verifies there is no more than one use of any **non-chain** output value.

With those two changes (and a few SystemZ backend updates), I am able to match all remaining versions of the extended-multiply instruction against constrained intrinsics.  This also fixes the last remaining set of test cases that were mixing strict and nonstrict operations in a single function.

The OPC_CheckFoldableChainNode change might theoretically affect code unrelated to strict FP nodes, but at least on SystemZ I could not find any single instance of that happening.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70913

Files:
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
  llvm/lib/Target/SystemZ/SystemZInstrFP.td
  llvm/test/CodeGen/SystemZ/fp-strict-mul-02.ll
  llvm/test/CodeGen/SystemZ/fp-strict-mul-04.ll
  llvm/test/CodeGen/SystemZ/vector-constrained-fp-intrinsics.ll
  llvm/test/CodeGen/X86/fp-intrinsics.ll
  llvm/test/CodeGen/X86/fp128-cast-strict.ll
  llvm/test/CodeGen/X86/fp128-libcalls-strict.ll
  llvm/test/CodeGen/X86/vector-constrained-fp-intrinsics.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D70913.231736.patch
Type: text/x-patch
Size: 120989 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191202/038ec6f7/attachment-0001.bin>