Commit 63336795 authored by Ulrich Weigand's avatar Ulrich Weigand
Browse files

[FPEnv] Default NoFPExcept SDNodeFlag to false 

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 makes it necessary to move
setting 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 matches on a set of orignal nodes
none of which can 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.)

Reviewed By: craig.topper

Differential Revision: https://reviews.llvm.org/D71841
parent 24ab9b53

llvm/include/llvm/CodeGen/ISDOpcodes.h

+6 −1
Original line number Diff line number Diff line
@@ -937,11 +937,16 @@ namespace ISD { 
    BUILTIN_OP_END

  };



  /// FIRST_TARGET_STRICTFP_OPCODE - Target-specific pre-isel operations

  /// which cannot raise FP exceptions should be less than this value.

  /// Those that do must not be less than this value.

  static const int FIRST_TARGET_STRICTFP_OPCODE = BUILTIN_OP_END+400;



  /// FIRST_TARGET_MEMORY_OPCODE - Target-specific pre-isel operations

  /// which do not reference a specific memory location should be less than

  /// this value. Those that do must not be less than this value, and can

  /// be used with SelectionDAG::getMemIntrinsicNode.

  static const int FIRST_TARGET_MEMORY_OPCODE = BUILTIN_OP_END+400;

  static const int FIRST_TARGET_MEMORY_OPCODE = BUILTIN_OP_END+500;



  //===--------------------------------------------------------------------===//

  /// MemIndexedMode enum - This enum defines the load / store indexed

llvm/include/llvm/CodeGen/SelectionDAGISel.h

+3 −0
Original line number Diff line number Diff line
@@ -310,6 +310,9 @@ public: 
    return false;

  }



  /// Return whether the node may raise an FP exception.

  bool mayRaiseFPException(SDNode *Node) const;



  bool isOrEquivalentToAdd(const SDNode *N) const;



private:

llvm/include/llvm/CodeGen/SelectionDAGNodes.h

+13 −4
Original line number Diff line number Diff line
@@ -387,7 +387,7 @@ public: 
        Exact(false), NoNaNs(false), NoInfs(false),

        NoSignedZeros(false), AllowReciprocal(false), VectorReduction(false),

        AllowContract(false), ApproximateFuncs(false),

        AllowReassociation(false), NoFPExcept(true) {}

        AllowReassociation(false), NoFPExcept(false) {}



  /// Propagate the fast-math-flags from an IR FPMathOperator.

  void copyFMF(const FPMathOperator &FPMO) {
@@ -450,9 +450,9 @@ public: 
    setDefined();

    AllowReassociation = b;

  }

  void setFPExcept(bool b) {

  void setNoFPExcept(bool b) {

    setDefined();

    NoFPExcept = !b;

    NoFPExcept = b;

  }



  // These are accessors for each flag.
@@ -467,7 +467,7 @@ public: 
  bool hasAllowContract() const { return AllowContract; }

  bool hasApproximateFuncs() const { return ApproximateFuncs; }

  bool hasAllowReassociation() const { return AllowReassociation; }

  bool hasFPExcept() const { return !NoFPExcept; }

  bool hasNoFPExcept() const { return NoFPExcept; }



  bool isFast() const {

    return NoSignedZeros && AllowReciprocal && NoNaNs && NoInfs && NoFPExcept &&
@@ -666,6 +666,15 @@ public: 
  /// \<target\>ISD namespace).

  bool isTargetOpcode() const { return NodeType >= ISD::BUILTIN_OP_END; }



  /// Test if this node has a target-specific opcode that may raise

  /// FP exceptions (in the \<target\>ISD namespace and greater than

  /// FIRST_TARGET_STRICTFP_OPCODE).  Note that all target memory

  /// opcode are currently automatically considered to possibly raise

  /// FP exceptions as well.

  bool isTargetStrictFPOpcode() const {

    return NodeType >= ISD::FIRST_TARGET_STRICTFP_OPCODE;

  }



  /// Test if this node has a target-specific

  /// memory-referencing opcode (in the \<target\>ISD namespace and

  /// greater than FIRST_TARGET_MEMORY_OPCODE).

llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp

+1 −1
Original line number Diff line number Diff line
@@ -882,7 +882,7 @@ EmitMachineNode(SDNode *Node, bool IsClone, bool IsCloned, 
    if (Flags.hasExact())

      MI->setFlag(MachineInstr::MIFlag::IsExact);



    if (Flags.hasFPExcept())

    if (MI->getDesc().mayRaiseFPException() && !Flags.hasNoFPExcept())

      MI->setFlag(MachineInstr::MIFlag::FPExcept);

  }

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

+9 −6
Original line number Diff line number Diff line
@@ -1108,6 +1108,15 @@ void SelectionDAGBuilder::visit(const Instruction &I) { 
        Node->intersectFlagsWith(IncomingFlags);

    }

  }

  // Constrained FP intrinsics with fpexcept.ignore should also get

  // the NoFPExcept flag.

  if (auto *FPI = dyn_cast<ConstrainedFPIntrinsic>(&I))

    if (FPI->getExceptionBehavior() == fp::ExceptionBehavior::ebIgnore)

      if (SDNode *Node = getNodeForIRValue(&I)) {

        SDNodeFlags Flags = Node->getFlags();

        Flags.setNoFPExcept(true);

        Node->setFlags(Flags);

      }



  if (!I.isTerminator() && !HasTailCall &&

      !isStatepoint(&I)) // statepoints handle their exports internally
@@ -6972,12 +6981,6 @@ void SelectionDAGBuilder::visitConstrainedFPIntrinsic( 
  SDVTList VTs = DAG.getVTList(ValueVTs);

  SDValue Result = DAG.getNode(Opcode, sdl, VTs, Opers);



  if (FPI.getExceptionBehavior() != fp::ExceptionBehavior::ebIgnore) {

    SDNodeFlags Flags;

    Flags.setFPExcept(true);

    Result->setFlags(Flags);

  }



  assert(Result.getNode()->getNumValues() == 2);

  // See above -- chain is handled like for loads here.

  SDValue OutChain = Result.getValue(1);