[llvm] 714ceef - [SelectionDAG] Always intersect SDNode flags during getNode() node memoization.
Jonas Paulsson via llvm-commits
llvm-commits at lists.llvm.org
Sat Sep 5 01:32:03 PDT 2020
Author: Jonas Paulsson
Date: 2020-09-05T10:30:38+02:00
New Revision: 714ceefad9b96ab3ef20913f2110883a1ad34a13
URL: https://github.com/llvm/llvm-project/commit/714ceefad9b96ab3ef20913f2110883a1ad34a13
DIFF: https://github.com/llvm/llvm-project/commit/714ceefad9b96ab3ef20913f2110883a1ad34a13.diff
LOG: [SelectionDAG] Always intersect SDNode flags during getNode() node memoization.
Previously SDNodeFlags::instersectWith(Flags) would do nothing if Flags was
in an undefined state, which is very bad given that this is the default when
getNode() is called without passing an explicit SDNodeFlags argument.
This meant that if an already existing and reused node had a flag which the
second caller to getNode() did not set, that flag would remain uncleared.
This was exposed by https://bugs.llvm.org/show_bug.cgi?id=47092, where an NSW
flag was incorrectly set on an add instruction (which did in fact overflow in
one of the two original contexts), so when SystemZElimCompare removed the
compare with 0 trusting that flag, wrong-code resulted.
There is more that needs to be done in this area as discussed here:
Differential Revision: https://reviews.llvm.org/D86871
Review: Ulrich Weigand, Sanjay Patel
Added:
llvm/test/CodeGen/SystemZ/fp-mul-14.ll
llvm/test/CodeGen/SystemZ/int-cmp-60.ll
Modified:
llvm/include/llvm/CodeGen/SelectionDAGNodes.h
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp
llvm/lib/Target/AMDGPU/AMDGPUISelLowering.h
Removed:
################################################################################
diff --git a/llvm/include/llvm/CodeGen/SelectionDAGNodes.h b/llvm/include/llvm/CodeGen/SelectionDAGNodes.h
index cde075f41f73..6eef79162f8a 100644
--- a/llvm/include/llvm/CodeGen/SelectionDAGNodes.h
+++ b/llvm/include/llvm/CodeGen/SelectionDAGNodes.h
@@ -357,9 +357,8 @@ template<> struct simplify_type<SDUse> {
/// the backend.
struct SDNodeFlags {
private:
- // This bit is used to determine if the flags are in a defined state.
- // Flag bits can only be masked out during intersection if the masking flags
- // are defined.
+ // This bit is used to determine if the flags are in a defined state. It is
+ // only used by SelectionDAGBuilder.
bool AnyDefined : 1;
bool NoUnsignedWrap : 1;
@@ -464,11 +463,9 @@ struct SDNodeFlags {
bool hasAllowReassociation() const { return AllowReassociation; }
bool hasNoFPExcept() const { return NoFPExcept; }
- /// Clear any flags in this flag set that aren't also set in Flags.
- /// If the given Flags are undefined then don't do anything.
+ /// Clear any flags in this flag set that aren't also set in Flags. All
+ /// flags will be cleared if Flags are undefined.
void intersectWith(const SDNodeFlags Flags) {
- if (!Flags.isDefined())
- return;
NoUnsignedWrap &= Flags.NoUnsignedWrap;
NoSignedWrap &= Flags.NoSignedWrap;
Exact &= Flags.Exact;
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index 1a2c77974c2b..5e6cb03f3839 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -1128,6 +1128,8 @@ void SelectionDAGBuilder::visit(const Instruction &I) {
// TODO: We could handle all flags (nsw, etc) here.
// TODO: If an IR instruction maps to >1 node, only the final node will have
// flags set.
+ // TODO: The handling of flags should be improved, see
+ // https://reviews.llvm.org/D86871
if (SDNode *Node = getNodeForIRValue(&I)) {
SDNodeFlags IncomingFlags;
IncomingFlags.copyFMF(*FPMO);
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp b/llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp
index 151b1bdd5538..5dd42d1f4a6a 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp
@@ -521,8 +521,8 @@ bool AMDGPUDAGToDAGISel::isNoNanSrc(SDValue N) const {
return true;
// TODO: Move into isKnownNeverNaN
- if (N->getFlags().isDefined())
- return N->getFlags().hasNoNaNs();
+ if (N->getFlags().hasNoNaNs())
+ return true;
return CurDAG->isKnownNeverNaN(N);
}
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUISelLowering.h b/llvm/lib/Target/AMDGPU/AMDGPUISelLowering.h
index c7fdc79c3b1a..932a05a4ba8c 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUISelLowering.h
+++ b/llvm/lib/Target/AMDGPU/AMDGPUISelLowering.h
@@ -150,8 +150,8 @@ class AMDGPUTargetLowering : public TargetLowering {
return true;
const auto Flags = Op.getNode()->getFlags();
- if (Flags.isDefined())
- return Flags.hasNoSignedZeros();
+ if (Flags.hasNoSignedZeros())
+ return true;
return false;
}
diff --git a/llvm/test/CodeGen/SystemZ/fp-mul-14.ll b/llvm/test/CodeGen/SystemZ/fp-mul-14.ll
new file mode 100644
index 000000000000..8bab2135739c
--- /dev/null
+++ b/llvm/test/CodeGen/SystemZ/fp-mul-14.ll
@@ -0,0 +1,20 @@
+; RUN: llc < %s -mtriple=s390x-linux-gnu | FileCheck %s
+;
+; Check that a multiply-and-add results.
+
+; FIXME: This test is xfailed temporarily
+; XFAIL: *
+
+define void @f1(float %arg, float* %Dst) {
+; CHECK-LABEL: f1:
+; CHECK: maeb
+bb:
+ %i = fmul contract float %arg, 0xBE6777A5C0000000
+ %i4 = fadd contract float %i, 1.000000e+00
+ %i5 = fmul contract float %arg, 0xBE6777A5C0000000
+ %i6 = fadd contract float %i5, 1.000000e+00
+ %i7 = fmul contract float %i4, 2.000000e+00
+ store float %i7, float* %Dst
+ ret void
+}
+
diff --git a/llvm/test/CodeGen/SystemZ/int-cmp-60.ll b/llvm/test/CodeGen/SystemZ/int-cmp-60.ll
new file mode 100644
index 000000000000..faae4f9bced2
--- /dev/null
+++ b/llvm/test/CodeGen/SystemZ/int-cmp-60.ll
@@ -0,0 +1,29 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=s390x-linux-gnu -mcpu=z14 | FileCheck %s
+;
+; Test that DAGCombiner properly clears the NUW/NSW flags on the memoized add
+; node.
+
+define void @fun(i64* %Src, i32* %Dst) {
+; CHECK-LABEL: fun:
+; CHECK: # %bb.0: # %entry
+; CHECK-NEXT: iilf %r0, 1303940520
+; CHECK-NEXT: n %r0, 4(%r2)
+; CHECK-NEXT: lr %r1, %r0
+; CHECK-NEXT: afi %r1, 1628135358
+; CHECK-NEXT: locrnhe %r1, %r0
+; CHECK-NEXT: st %r1, 0(%r3)
+; CHECK-NEXT: br %r14
+entry:
+ %0 = load i64, i64* %Src, align 8
+ %1 = trunc i64 %0 to i32
+ %conv = and i32 %1, 1303940520
+ %xor11.i = or i32 %conv, -2147483648
+ %xor2.i = add i32 %xor11.i, -519348290
+ %cmp.i = icmp slt i32 %xor2.i, 0
+ %sub3.i = add nuw nsw i32 %conv, 1628135358
+ %cond.i = select i1 %cmp.i, i32 %conv, i32 %sub3.i
+ store i32 %cond.i, i32* %Dst
+ ret void
+}
+
More information about the llvm-commits
mailing list