[PATCH] D71841: [FPEnv] Default NoFPExcept SDNodeFlag to false

[Ulrich Weigand via Phabricator via llvm-commits]

uweigand created this revision.
uweigand added reviewers: craig.topper, andrew.w.kaylor, cameron.mcinally, kpn, RKSimon.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.
uweigand added a comment.
uweigand added a parent revision: D71840: [FPEnv][X86] More strict int <-> FP conversion fixes.

Note that this patch has D71840 <https://reviews.llvm.org/D71840> as prerequisite.


The NoFPExcept bit in SDNodeFlags currently defaults to true, unlike all other such flags.  This is a problem, because it implies that all code that transforms SDNodes without copying flags can introduce a **correctness** bug, not just a missed optimization.

This patch changes the default to false.   This requires moving the setting of the (No)FPExcept flag for constrained intrinsics from the visitConstrainedIntrinsic routine to the generic visit routine at the place where the other flags are set, or else the intersectFlagsWith call would erase the NoFPExcept flag again.

In order to avoid making non-strict FP code worse, whenever SelectionDAGISel::SelectCodeCommon is called on an original node that can **not** raise FP exceptions, it will preserve this property on all results nodes generated, by setting the NoFPExcept flag on those result nodes that would otherwise be considered as raising an FP exception.

To check whether or not an SD node should be considered as raising an FP exception, the following logic applies:

- For machine nodes, check the mayRaiseFPException property of the underlying MI instruction
- For regular nodes, check isStrictFPOpcode
- For target nodes, check a newly introduced isTargetStrictFPOpcode

The latter is implemented by reserving a range of target opcodes, similarly to how memory opcodes are identified.  (Note that there a bit of a quirk in identifying target nodes that are **both** memory nodes and strict FP nodes.  To simplify the logic, right now all target memory nodes are automatically also considered strict FP nodes -- this could be fixed by adding one more range.)

I've updated SystemZ and X86 (the only targets currently defining target strict FP nodes) accordingly.

This patch causes a number of changes to X86 test cases, but most of them are simply that instructions are now (correctly) marked fpexcept at the MI level, where this was missed by old code.  In one case this led to codegen changes (different register allocation) since some copy propagation is no longer run on instructions marked as fpexcept.

Note that the MI-level fpexcept flag is not inverted by this patch.  For correctness reasons, this should still be done as well; I'm working on a follow-on patch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71841

Files:
  llvm/include/llvm/CodeGen/ISDOpcodes.h
  llvm/include/llvm/CodeGen/SelectionDAGISel.h
  llvm/include/llvm/CodeGen/SelectionDAGNodes.h
  llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGDumper.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
  llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
  llvm/lib/Target/SystemZ/SystemZISelLowering.h
  llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h
  llvm/test/CodeGen/X86/fp-intrinsics-flags-x86_64.ll
  llvm/test/CodeGen/X86/fp-intrinsics-flags.ll
  llvm/test/CodeGen/X86/vec-strict-fptoint-256.ll
  llvm/test/CodeGen/X86/vector-constrained-fp-intrinsics-flags.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D71841.235139.patch
Type: text/x-patch
Size: 28075 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191223/6fa44e04/attachment.bin>