ValueTracking: Check instruction is in a parent in computeKnownFPClass
authorMatt Arsenault <Matthew.Arsenault@amd.com>
Thu, 18 May 2023 11:16:04 +0000 (12:16 +0100)
committerMatt Arsenault <arsenm2@gmail.com>
Thu, 18 May 2023 11:21:47 +0000 (12:21 +0100)
For some reason the inliner calls simplifyInstruction with disembodied
instructions. I consider this to be an API defect. Either the instruction
should always be inserted prior to simplification, or we at least
should pass in the new function for the context.

llvm/lib/Analysis/ValueTracking.cpp
llvm/lib/Transforms/Utils/CloneFunction.cpp
llvm/test/Transforms/Inline/simplify-instruction-computeKnownFPClass-context.ll [new file with mode: 0644]

index 3b70029..089d560 100644 (file)
@@ -4540,8 +4540,8 @@ void computeKnownFPClass(const Value *V, const APInt &DemandedElts,
 
         // If the input denormal mode could be PreserveSign, a negative
         // subnormal input could produce a negative zero output.
-        if (KnownSrc.isKnownNeverLogicalNegZero(*II->getFunction(),
-                                                II->getType())) {
+        const Function *F = II->getFunction();
+        if (F && KnownSrc.isKnownNeverLogicalNegZero(*F, II->getType())) {
           Known.knownNot(fcNegZero);
           if (KnownSrc.isKnownNeverNaN())
             Known.SignBit = false;
@@ -4634,11 +4634,15 @@ void computeKnownFPClass(const Value *V, const APInt &DemandedElts,
         // Canonicalize is guaranteed to quiet signaling nans.
         Known.knownNot(fcSNan);
 
+        const Function *F = II->getFunction();
+        if (!F)
+          break;
+
         // If the parent function flushes denormals, the canonical output cannot
         // be a denormal.
         const fltSemantics &FPType =
             II->getType()->getScalarType()->getFltSemantics();
-        DenormalMode DenormMode = II->getFunction()->getDenormalMode(FPType);
+        DenormalMode DenormMode = F->getDenormalMode(FPType);
         if (DenormMode.inputsAreZero() || DenormMode.outputsAreZero())
           Known.knownNot(fcSubnormal);
 
@@ -4738,7 +4742,8 @@ void computeKnownFPClass(const Value *V, const APInt &DemandedElts,
             KnownSrc.cannotBeOrderedLessThanZero())
           Known.knownNot(fcNan);
 
-        if (KnownSrc.isKnownNeverLogicalZero(*II->getFunction(), II->getType()))
+        const Function *F = II->getFunction();
+        if (F && KnownSrc.isKnownNeverLogicalZero(*F, II->getType()))
           Known.knownNot(fcNegInf);
 
         break;
@@ -4819,7 +4824,11 @@ void computeKnownFPClass(const Value *V, const APInt &DemandedElts,
           (KnownLHS.isKnownNeverInfinity() || KnownRHS.isKnownNeverInfinity()))
         Known.knownNot(fcNan);
 
+      // FIXME: Context function should always be passed in separately
       const Function *F = cast<Instruction>(Op)->getFunction();
+      if (!F)
+        break;
+
       if (Op->getOpcode() == Instruction::FAdd) {
         // (fadd x, 0.0) is guaranteed to return +0.0, not -0.0.
         if ((KnownLHS.isKnownNeverLogicalNegZero(*F, Op->getType()) ||
@@ -4864,9 +4873,9 @@ void computeKnownFPClass(const Value *V, const APInt &DemandedElts,
       // TODO: Check operand combinations.
       // e.g. fmul nofpclass(inf nan zero), nofpclass(nan) -> nofpclass(nan)
       if ((KnownLHS.isKnownNeverInfinity() ||
-           KnownLHS.isKnownNeverLogicalZero(*F, Op->getType())) &&
+           (F && KnownLHS.isKnownNeverLogicalZero(*F, Op->getType()))) &&
           (KnownRHS.isKnownNeverInfinity() ||
-           KnownRHS.isKnownNeverLogicalZero(*F, Op->getType())))
+           (F && KnownRHS.isKnownNeverLogicalZero(*F, Op->getType()))))
         Known.knownNot(fcNan);
     }
 
@@ -4920,8 +4929,8 @@ void computeKnownFPClass(const Value *V, const APInt &DemandedElts,
       if (KnownLHS.isKnownNeverNaN() && KnownRHS.isKnownNeverNaN() &&
           (KnownLHS.isKnownNeverInfinity() ||
            KnownRHS.isKnownNeverInfinity()) &&
-          (KnownLHS.isKnownNeverLogicalZero(*F, Op->getType()) ||
-           KnownRHS.isKnownNeverLogicalZero(*F, Op->getType()))) {
+          ((F && KnownLHS.isKnownNeverLogicalZero(*F, Op->getType())) ||
+           (F && KnownRHS.isKnownNeverLogicalZero(*F, Op->getType())))) {
         Known.knownNot(fcNan);
       }
 
@@ -4932,7 +4941,7 @@ void computeKnownFPClass(const Value *V, const APInt &DemandedElts,
     } else {
       // Inf REM x and x REM 0 produce NaN.
       if (KnownLHS.isKnownNeverNaN() && KnownRHS.isKnownNeverNaN() &&
-          KnownLHS.isKnownNeverInfinity() &&
+          KnownLHS.isKnownNeverInfinity() && F &&
           KnownRHS.isKnownNeverLogicalZero(*F, Op->getType())) {
         Known.knownNot(fcNan);
       }
index 315263a..272970e 100644 (file)
@@ -516,6 +516,8 @@ void PruningFunctionCloner::CloneBlock(
       // If we can simplify this instruction to some other value, simply add
       // a mapping to that value rather than inserting a new instruction into
       // the basic block.
+      //
+      // FIXME: simplifyInstruction should know the context of the new function.
       if (Value *V =
               simplifyInstruction(NewInst, BB->getModule()->getDataLayout())) {
         // On the off-chance that this simplifies to an instruction in the old
diff --git a/llvm/test/Transforms/Inline/simplify-instruction-computeKnownFPClass-context.ll b/llvm/test/Transforms/Inline/simplify-instruction-computeKnownFPClass-context.ll
new file mode 100644 (file)
index 0000000..06d08e1
--- /dev/null
@@ -0,0 +1,140 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 2
+; RUN: opt < %s -passes=inline -inline-threshold=20 -S | FileCheck %s
+; RUN: opt < %s -passes='cgscc(inline)' -inline-threshold=20 -S | FileCheck %s
+
+; Make sure there are no crashes when calling computeKnownFPClass with
+; un-inserted cloned instructions.
+
+; Hit computeKnownFPClass in a context where the denormal mode is
+; queried for the function for an operand not in a parent function.
+
+define i1 @simplify_fcmp_ord_fsub_caller(double nofpclass(zero nan) %i0, double nofpclass(zero nan) %i1) {
+; CHECK-LABEL: define i1 @simplify_fcmp_ord_fsub_caller
+; CHECK-SAME: (double nofpclass(nan zero) [[I0:%.*]], double nofpclass(nan zero) [[I1:%.*]]) {
+; CHECK-NEXT:    [[SUB_DOUBLE_SUB_I:%.*]] = fsub double [[I0]], [[I1]]
+; CHECK-NEXT:    [[CMP_I:%.*]] = fcmp ord double [[SUB_DOUBLE_SUB_I]], 0.000000e+00
+; CHECK-NEXT:    ret i1 [[CMP_I]]
+;
+  %call = call i1 @simplify_fcmp_ord_fsub_callee(double %i0, double %i1)
+  ret i1 %call
+}
+
+; Make sure we hit the denormal query in isKnownNeverLogicalZero
+define internal i1 @simplify_fcmp_ord_fsub_callee(double %a, double %b) {
+  %sub.double.sub = fsub double %a, %b
+  %cmp = fcmp ord double %sub.double.sub, 0.0
+  ret i1 %cmp
+}
+
+define i1 @simplify_fcmp_ord_fdiv_caller(double nofpclass(zero nan inf) %i0, double nofpclass(zero nan inf) %i1) {
+; CHECK-LABEL: define i1 @simplify_fcmp_ord_fdiv_caller
+; CHECK-SAME: (double nofpclass(nan inf zero) [[I0:%.*]], double nofpclass(nan inf zero) [[I1:%.*]]) {
+; CHECK-NEXT:    [[SUB_DOUBLE_SUB_I:%.*]] = fdiv double [[I0]], [[I1]]
+; CHECK-NEXT:    [[CMP_I:%.*]] = fcmp ord double [[SUB_DOUBLE_SUB_I]], 0.000000e+00
+; CHECK-NEXT:    ret i1 [[CMP_I]]
+;
+  %call = call i1 @simplify_fcmp_ord_fdiv_callee(double %i0, double %i1)
+  ret i1 %call
+}
+
+; Make sure we hit the denormal query in isKnownNeverLogicalZero
+define internal i1 @simplify_fcmp_ord_fdiv_callee(double %a, double %b) {
+  %sub.double.sub = fdiv double %a, %b
+  %cmp = fcmp ord double %sub.double.sub, 0.0
+  ret i1 %cmp
+}
+
+define i1 @simplify_fcmp_ord_frem_caller(double nofpclass(zero nan inf) %i0, double nofpclass(zero nan inf) %i1) {
+; CHECK-LABEL: define i1 @simplify_fcmp_ord_frem_caller
+; CHECK-SAME: (double nofpclass(nan inf zero) [[I0:%.*]], double nofpclass(nan inf zero) [[I1:%.*]]) {
+; CHECK-NEXT:    [[SUB_DOUBLE_SUB_I:%.*]] = frem double [[I0]], [[I1]]
+; CHECK-NEXT:    [[CMP_I:%.*]] = fcmp ord double [[SUB_DOUBLE_SUB_I]], 0.000000e+00
+; CHECK-NEXT:    ret i1 [[CMP_I]]
+;
+  %call = call i1 @simplify_fcmp_ord_frem_callee(double %i0, double %i1)
+  ret i1 %call
+}
+
+; Make sure we hit the denormal query in isKnownNeverLogicalZero
+define internal i1 @simplify_fcmp_ord_frem_callee(double %a, double %b) {
+  %sub.double.sub = frem double %a, %b
+  %cmp = fcmp ord double %sub.double.sub, 0.0
+  ret i1 %cmp
+}
+
+define i1 @simplify_fcmp_ord_fmul_caller(double nofpclass(zero nan) %i0, double nofpclass(zero nan) %i1) {
+; CHECK-LABEL: define i1 @simplify_fcmp_ord_fmul_caller
+; CHECK-SAME: (double nofpclass(nan zero) [[I0:%.*]], double nofpclass(nan zero) [[I1:%.*]]) {
+; CHECK-NEXT:    [[SUB_DOUBLE_SUB_I:%.*]] = fmul double [[I0]], [[I1]]
+; CHECK-NEXT:    [[CMP_I:%.*]] = fcmp ord double [[SUB_DOUBLE_SUB_I]], 0.000000e+00
+; CHECK-NEXT:    ret i1 [[CMP_I]]
+;
+  %call = call i1 @simplify_fcmp_ord_fmul_callee(double %i0, double %i1)
+  ret i1 %call
+}
+
+; Make sure we hit the denormal query in isKnownNeverLogicalZero
+define internal i1 @simplify_fcmp_ord_fmul_callee(double %a, double %b) {
+  %sub.double.sub = fmul double %a, %b
+  %cmp = fcmp ord double %sub.double.sub, 0.0
+  ret i1 %cmp
+}
+
+define i1 @simplify_fcmp_ord_sqrt_caller(double nofpclass(zero nan) %i0) {
+; CHECK-LABEL: define i1 @simplify_fcmp_ord_sqrt_caller
+; CHECK-SAME: (double nofpclass(nan zero) [[I0:%.*]]) {
+; CHECK-NEXT:    [[SUB_DOUBLE_SUB_I:%.*]] = call double @llvm.sqrt.f64(double [[I0]])
+; CHECK-NEXT:    [[CMP_I:%.*]] = fcmp ord double [[SUB_DOUBLE_SUB_I]], 0.000000e+00
+; CHECK-NEXT:    ret i1 [[CMP_I]]
+;
+  %call = call i1 @simplify_fcmp_ord_sqrt_callee(double %i0)
+  ret i1 %call
+}
+
+; Make sure we hit the denormal query in isKnownNeverLogicalZero
+define internal i1 @simplify_fcmp_ord_sqrt_callee(double %a) {
+  %sub.double.sub = call double @llvm.sqrt.f64(double %a)
+  %cmp = fcmp ord double %sub.double.sub, 0.0
+  ret i1 %cmp
+}
+
+declare double @llvm.sqrt.f64(double)
+
+define i1 @simplify_fcmp_ord_canonicalize_caller(double nofpclass(zero nan) %i0) {
+; CHECK-LABEL: define i1 @simplify_fcmp_ord_canonicalize_caller
+; CHECK-SAME: (double nofpclass(nan zero) [[I0:%.*]]) {
+; CHECK-NEXT:    [[SUB_DOUBLE_SUB_I:%.*]] = call double @llvm.canonicalize.f64(double [[I0]])
+; CHECK-NEXT:    ret i1 true
+;
+  %call = call i1 @simplify_fcmp_ord_canonicalize_callee(double %i0)
+  ret i1 %call
+}
+
+; Make sure we hit the denormal query in isKnownNeverLogicalZero
+define internal i1 @simplify_fcmp_ord_canonicalize_callee(double %a) {
+  %sub.double.sub = call double @llvm.canonicalize.f64(double %a)
+  %cmp = fcmp ord double %sub.double.sub, 0.0
+  ret i1 %cmp
+}
+
+declare double @llvm.canonicalize.f64(double)
+
+define i1 @simplify_fcmp_ord_log_caller(double nofpclass(zero nan) %i0) {
+; CHECK-LABEL: define i1 @simplify_fcmp_ord_log_caller
+; CHECK-SAME: (double nofpclass(nan zero) [[I0:%.*]]) {
+; CHECK-NEXT:    [[SUB_DOUBLE_SUB_I:%.*]] = call double @llvm.log.f64(double [[I0]])
+; CHECK-NEXT:    [[CMP_I:%.*]] = fcmp ord double [[SUB_DOUBLE_SUB_I]], 0.000000e+00
+; CHECK-NEXT:    ret i1 [[CMP_I]]
+;
+  %call = call i1 @simplify_fcmp_ord_log_callee(double %i0)
+  ret i1 %call
+}
+
+; Make sure we hit the denormal query in isKnownNeverLogicalZero
+define internal i1 @simplify_fcmp_ord_log_callee(double %a) {
+  %sub.double.sub = call double @llvm.log.f64(double %a)
+  %cmp = fcmp ord double %sub.double.sub, 0.0
+  ret i1 %cmp
+}
+
+declare double @llvm.log.f64(double)