diff options
| author | Ulrich Weigand <ulrich.weigand@de.ibm.com> | 2020-01-02 16:54:49 +0100 |
|---|---|---|
| committer | Ulrich Weigand <ulrich.weigand@de.ibm.com> | 2020-01-02 16:59:45 +0100 |
| commit | 63336795f0d50a009e8ec034d95811170efc978b (patch) | |
| tree | 57bad3c045728b5e88b35af479654ee5bd4b8664 /llvm/lib/CodeGen | |
| parent | 24ab9b537e61b3fe5e6a1019492ff6530d82a3ee (diff) | |
| download | bcm5719-llvm-63336795f0d50a009e8ec034d95811170efc978b.tar.gz bcm5719-llvm-63336795f0d50a009e8ec034d95811170efc978b.zip | |
[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
Diffstat (limited to 'llvm/lib/CodeGen')
5 files changed, 49 insertions, 10 deletions
diff --git a/llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp b/llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp index c5095995ec2..a0c8e83cd8a 100644 --- a/llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp @@ -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); } diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp index f71ad863558..b78b8e93435 100644 --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp @@ -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); diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGDumper.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGDumper.cpp index 92e4160d69d..b5018ec2111 100644 --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGDumper.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGDumper.cpp @@ -547,8 +547,8 @@ void SDNode::print_details(raw_ostream &OS, const SelectionDAG *G) const { if (getFlags().hasVectorReduction()) OS << " vector-reduction"; - if (getFlags().hasFPExcept()) - OS << " fpexcept"; + if (getFlags().hasNoFPExcept()) + OS << " nofpexcept"; if (const MachineSDNode *MN = dyn_cast<MachineSDNode>(this)) { if (!MN->memoperands_empty()) { diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp index 8317a11caa0..62e519438ca 100644 --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp @@ -3458,6 +3458,17 @@ void SelectionDAGISel::SelectCodeCommon(SDNode *NodeToMatch, if ((EmitNodeInfo & OPFL_GlueInput) && InputGlue.getNode() != nullptr) Ops.push_back(InputGlue); + // Check whether any matched node could raise an FP exception. Since all + // such nodes must have a chain, it suffices to check ChainNodesMatched. + // We need to perform this check before potentially modifying one of the + // nodes via MorphNode. + bool MayRaiseFPException = false; + for (auto *N : ChainNodesMatched) + if (mayRaiseFPException(N) && !N->getFlags().hasNoFPExcept()) { + MayRaiseFPException = true; + break; + } + // Create the node. MachineSDNode *Res = nullptr; bool IsMorphNodeTo = Opcode == OPC_MorphNodeTo || @@ -3489,6 +3500,14 @@ void SelectionDAGISel::SelectCodeCommon(SDNode *NodeToMatch, Ops, EmitNodeInfo)); } + // Set the NoFPExcept flag when no original matched node could + // raise an FP exception, but the new node potentially might. + if (!MayRaiseFPException && mayRaiseFPException(Res)) { + SDNodeFlags Flags = Res->getFlags(); + Flags.setNoFPExcept(true); + Res->setFlags(Flags); + } + // If the node had chain/glue results, update our notion of the current // chain and glue. if (EmitNodeInfo & OPFL_GlueOutput) { @@ -3644,6 +3663,21 @@ void SelectionDAGISel::SelectCodeCommon(SDNode *NodeToMatch, } } +/// Return whether the node may raise an FP exception. +bool SelectionDAGISel::mayRaiseFPException(SDNode *N) const { + // For machine opcodes, consult the MCID flag. + if (N->isMachineOpcode()) { + const MCInstrDesc &MCID = TII->get(N->getMachineOpcode()); + return MCID.mayRaiseFPException(); + } + + // For ISD opcodes, only StrictFP opcodes may raise an FP + // exception. + if (N->isTargetOpcode()) + return N->isTargetStrictFPOpcode(); + return N->isStrictFPOpcode(); +} + bool SelectionDAGISel::isOrEquivalentToAdd(const SDNode *N) const { assert(N->getOpcode() == ISD::OR && "Unexpected opcode"); auto *C = dyn_cast<ConstantSDNode>(N->getOperand(1)); diff --git a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp index 0bfd1b62db2..1049a897c6f 100644 --- a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp @@ -6190,8 +6190,10 @@ bool TargetLowering::expandUINT_TO_FP(SDNode *Node, SDValue &Result, // incoming STRICT_UINT_TO_FP node; the STRICT_FADD node can // never raise any exception. SDNodeFlags Flags; - Flags.setFPExcept(Node->getFlags().hasFPExcept()); + Flags.setNoFPExcept(Node->getFlags().hasNoFPExcept()); Fast->setFlags(Flags); + Flags.setNoFPExcept(true); + Slow->setFlags(Flags); } else { SDValue SignCvt = DAG.getNode(ISD::SINT_TO_FP, dl, DstVT, Or); Slow = DAG.getNode(ISD::FADD, dl, DstVT, SignCvt, SignCvt); |

