Add FMF to hasPoisonGeneratingFlags/dropPoisonGeneratingFlags

[reames]

Summary

These flags are documented as generating poison values for particular input values. As such, we should really be consistent about their handling with how we handle nsw/nuw/exact/inbounds.

In terms of the test changes, a couple notes:

• I did a bunch of cleanup in previous change series to reduce the test diff. Make sure you apply to ToT if applying locally.
• The freeze changes are correct, and desired.
• The shuffle changes look to be over conservative to me, but glancing at the history, it appears that's a complicated area. I think we should take the potential regression in quality here to have the consistency, and revisit the basic question of where we need to drop flags separately.

[nikic]

Can you please rebase now that D115526 has landed?

[reames]

Rebase as requested.

[spatel]

llvm/test/Transforms/InstCombine/shuffle_select-inseltpoison.ll

779

I looked at the code for this and related transforms in "foldSelectShuffle", and it's a mess.
We have lots of miscompiles like this:
https://alive2.llvm.org/ce/z/MHoacO
...and I don't think proposed but uncommitted patches like D93818 / D103874 resolve those.

We probably need to add an identity shuffle with undef/poison elements preserved from the original code. Otherwise, we are potentially leaking poison from the variable value in the original code. This could take a few patches to fix, so I wouldn't hold this patch up for it (but I'll try to get that sorted out soon).

llvm/test/Transforms/PhaseOrdering/ARM/mve-floatreduce.ll

16 ↗

(On Diff #393922)

This diff should have disappeared with the rebase?

[reames]

llvm/test/Transforms/InstCombine/shuffle_select-inseltpoison.ll

779

I don't follow this comment at all, so if there's anything in here you want me to change, please rephrase.

llvm/test/Transforms/PhaseOrdering/ARM/mve-floatreduce.ll

16 ↗

(On Diff #393922)

This passed check-llvm after the rebase, and the CI build checks agree, so no.

[spatel]

llvm/test/Transforms/InstCombine/shuffle_select-inseltpoison.ll

779

Sorry, that was confusing. There are problems with this set of transforms that are independent of this patch. The test changes here are minor, so I am not worried about them. Nothing for you to do in this patch.

llvm/test/Transforms/PhaseOrdering/ARM/mve-floatreduce.ll

16 ↗

(On Diff #393922)

Hmm...if I apply this patch locally, this test fails for me...

; CHECK-NEXT: [[TMP5:%.*]] = fadd reassoc nsz arcp contract afn <8 x half> [[TMP1]], [[TMP4]]
              ^
<stdin>:13:41: note: scanning from here
 %4 = bitcast <4 x i32> %3 to <8 x half>
                                        ^
<stdin>:13:41: note: with "TMP1" equal to "%1"
 %4 = bitcast <4 x i32> %3 to <8 x half>
                                        ^
<stdin>:13:41: note: with "TMP4" equal to "%4"
 %4 = bitcast <4 x i32> %3 to <8 x half>
                                        ^
<stdin>:14:2: note: possible intended match here
 %5 = fadd fast <8 x half> %1, %4
 ^

Input file: <stdin>
Check file: /Users/spatel/GitHub/llvm-project/llvm/test/Transforms/PhaseOrdering/ARM/mve-floatreduce.ll

-dump-input=help explains the following input dump.

Input was:
<<<<<<
           .
           .
           .
           8: entry: 
           9:  %0 = shufflevector <8 x half> %in, <8 x half> poison, <8 x i32> <i32 1, i32 0, i32 3, i32 2, i32 5, i32 4, i32 7, i32 6> 
          10:  %1 = fadd fast <8 x half> %0, %in 
          11:  %2 = bitcast <8 x half> %1 to <4 x i32> 
          12:  %3 = shufflevector <4 x i32> %2, <4 x i32> poison, <4 x i32> <i32 1, i32 undef, i32 3, i32 undef> 
          13:  %4 = bitcast <4 x i32> %3 to <8 x half> 
next:16'0                                             X error: no match found
next:16'1                                               with "TMP1" equal to "%1"
next:16'2                                               with "TMP4" equal to "%4"
          14:  %5 = fadd fast <8 x half> %1, %4

[spatel]

llvm/test/Transforms/PhaseOrdering/ARM/mve-floatreduce.ll

16 ↗

(On Diff #393922)

And it failed on this bot if I'm reading the log correctly:
https://buildkite.com/llvm-project/premerge-checks/builds/69791#42cc4b83-6e07-4217-a3bc-6e3a38fc7cae

[reames]

2nd try at a rebase

This *definitely* passes check-llvm.

[reames]

llvm/test/Transforms/PhaseOrdering/ARM/mve-floatreduce.ll

16 ↗

(On Diff #393922)

Did a second rebase cycle, and the diff appears to have disappeared. Clearly must have done some wrong the first time, not really sure what though.

[spatel]

LGTM - as I mentioned in an inline comment, the remaining regressions seem minor, and I will try to fix them in follow-up patches.

[nikic]

LG as well, thanks for following up on this.

llvm/lib/Analysis/ValueTracking.cpp

 }
 
 static bool canCreateUndefOrPoison(const Operator *Op, bool PoisonOnly,
                                    bool ConsiderFlags) {
 
   if (ConsiderFlags && Op->hasPoisonGeneratingFlags())
     return true;
 
-  // TODO: this should really be under the ConsiderFlags block, but currently
-  // these are not dropped by dropPoisonGeneratingFlags
-  if (const auto *FP = dyn_cast<FPMathOperator>(Op)) {
-    auto FMF = FP->getFastMathFlags();
-    if (FMF.noNaNs() || FMF.noInfs())
-      return true;
-  }
-
   unsigned Opcode = Op->getOpcode();
 
   // Check whether opcode is a poison/undef-generating operation
   switch (Opcode) {
   case Instruction::Shl:
   case Instruction::AShr:
   case Instruction::LShr: {
     // Shifts return poison if shiftwidth is larger than the bitwidth.

llvm/lib/IR/Instruction.cpp

void Instruction::dropPoisonGeneratingFlags() {
   case Instruction::LShr:
     cast<PossiblyExactOperator>(this)->setIsExact(false);
     break;
 
   case Instruction::GetElementPtr:
     cast<GetElementPtrInst>(this)->setIsInBounds(false);
     break;
   }
-  // TODO: FastMathFlags!
+  if (isa<FPMathOperator>(this)) {
+    setHasNoNaNs(false);
+    setHasNoInfs(false);
+  }
 
   assert(!hasPoisonGeneratingFlags() && "must be kept in sync");
 }
 
 void Instruction::dropUndefImplyingAttrsAndUnknownMetadata(
     ArrayRef<unsigned> KnownIDs) {
   dropUnknownNonDebugMetadata(KnownIDs);
   auto *CB = dyn_cast<CallBase>(this);

llvm/lib/IR/Operator.cpp

bool Operator::hasPoisonGeneratingFlags() const {
   case Instruction::LShr:
     return cast<PossiblyExactOperator>(this)->isExact();
   case Instruction::GetElementPtr: {
     auto *GEP = cast<GEPOperator>(this);
     // Note: inrange exists on constexpr only
     return GEP->isInBounds() || GEP->getInRangeIndex() != None;
   }
   default:
+    if (const auto *FP = dyn_cast<FPMathOperator>(this))
+      return FP->hasNoNaNs() || FP->hasNoInfs();
     return false;
   }
-  // TODO: FastMathFlags!  (On instructions, but not constexpr)
 }
 
 Type *GEPOperator::getSourceElementType() const {
   if (auto *I = dyn_cast<GetElementPtrInst>(this))
     return I->getSourceElementType();
   return cast<GetElementPtrConstantExpr>(this)->getSourceElementType();
 }
 

llvm/test/Transforms/InstCombine/freeze.ll

 ;
   %v1 = getelementptr inbounds i8, i8* %arg, i64 16
   %v1.fr = freeze i8* %v1
   ret i8* %v1.fr
 }
 
 define float @propagate_drop_fneg(float %arg) {
 ; CHECK-LABEL: @propagate_drop_fneg(
-; CHECK-NEXT:    [[V1:%.*]] = fneg nnan ninf float [[ARG:%.*]]
-; CHECK-NEXT:    [[V1_FR:%.*]] = freeze float [[V1]]
-; CHECK-NEXT:    ret float [[V1_FR]]
+; CHECK-NEXT:    [[ARG_FR:%.*]] = freeze float [[ARG:%.*]]
+; CHECK-NEXT:    [[V1:%.*]] = fneg float [[ARG_FR]]
+; CHECK-NEXT:    ret float [[V1]]
 ;
   %v1 = fneg ninf nnan float %arg
   %v1.fr = freeze float %v1
   ret float %v1.fr
 }
 
 
 define float @propagate_drop_fadd(float %arg) {
 ; CHECK-LABEL: @propagate_drop_fadd(
-; CHECK-NEXT:    [[V1:%.*]] = fadd nnan ninf float [[ARG:%.*]], 2.000000e+00
-; CHECK-NEXT:    [[V1_FR:%.*]] = freeze float [[V1]]
-; CHECK-NEXT:    ret float [[V1_FR]]
+; CHECK-NEXT:    [[ARG_FR:%.*]] = freeze float [[ARG:%.*]]
+; CHECK-NEXT:    [[V1:%.*]] = fadd float [[ARG_FR]], 2.000000e+00
+; CHECK-NEXT:    ret float [[V1]]
 ;
   %v1 = fadd ninf nnan float %arg, 2.0
   %v1.fr = freeze float %v1
   ret float %v1.fr
 }
 
 define float @propagate_drop_fsub(float %arg) {
 ; CHECK-LABEL: @propagate_drop_fsub(
-; CHECK-NEXT:    [[V1:%.*]] = fadd nnan ninf float [[ARG:%.*]], -2.000000e+00
-; CHECK-NEXT:    [[V1_FR:%.*]] = freeze float [[V1]]
-; CHECK-NEXT:    ret float [[V1_FR]]
+; CHECK-NEXT:    [[ARG_FR:%.*]] = freeze float [[ARG:%.*]]
+; CHECK-NEXT:    [[V1:%.*]] = fadd float [[ARG_FR]], -2.000000e+00
+; CHECK-NEXT:    ret float [[V1]]
 ;
   %v1 = fsub ninf nnan float %arg, 2.0
   %v1.fr = freeze float %v1
   ret float %v1.fr
 }
 
 define float @propagate_drop_fmul(float %arg) {
 ; CHECK-LABEL: @propagate_drop_fmul(
-; CHECK-NEXT:    [[V1:%.*]] = fmul nnan ninf float [[ARG:%.*]], 2.000000e+00
-; CHECK-NEXT:    [[V1_FR:%.*]] = freeze float [[V1]]
-; CHECK-NEXT:    ret float [[V1_FR]]
+; CHECK-NEXT:    [[ARG_FR:%.*]] = freeze float [[ARG:%.*]]
+; CHECK-NEXT:    [[V1:%.*]] = fmul float [[ARG_FR]], 2.000000e+00
+; CHECK-NEXT:    ret float [[V1]]
 ;
   %v1 = fmul ninf nnan float %arg, 2.0
   %v1.fr = freeze float %v1
   ret float %v1.fr
 }
 
 define float @propagate_drop_fdiv(float %arg) {
 ; CHECK-LABEL: @propagate_drop_fdiv(
-; CHECK-NEXT:    [[V1:%.*]] = fmul nnan ninf float [[ARG:%.*]], 5.000000e-01
-; CHECK-NEXT:    [[V1_FR:%.*]] = freeze float [[V1]]
-; CHECK-NEXT:    ret float [[V1_FR]]
+; CHECK-NEXT:    [[ARG_FR:%.*]] = freeze float [[ARG:%.*]]
+; CHECK-NEXT:    [[V1:%.*]] = fmul float [[ARG_FR]], 5.000000e-01
+; CHECK-NEXT:    ret float [[V1]]
 ;
   %v1 = fdiv ninf nnan float %arg, 2.0
   %v1.fr = freeze float %v1
   ret float %v1.fr
 }
 
 define float @propagate_drop_frem(float %arg) {
 ; CHECK-LABEL: @propagate_drop_frem(
-; CHECK-NEXT:    [[V1:%.*]] = frem nnan ninf float [[ARG:%.*]], 2.000000e+00
-; CHECK-NEXT:    [[V1_FR:%.*]] = freeze float [[V1]]
-; CHECK-NEXT:    ret float [[V1_FR]]
+; CHECK-NEXT:    [[ARG_FR:%.*]] = freeze float [[ARG:%.*]]
+; CHECK-NEXT:    [[V1:%.*]] = frem float [[ARG_FR]], 2.000000e+00
+; CHECK-NEXT:    ret float [[V1]]
 ;
   %v1 = frem ninf nnan float %arg, 2.0
   %v1.fr = freeze float %v1
   ret float %v1.fr
 }
 
 define i1 @propagate_drop_fcmp(float %arg) {
 ; CHECK-LABEL: @propagate_drop_fcmp(
-; CHECK-NEXT:    [[V1:%.*]] = fcmp nnan ninf une float [[ARG:%.*]], 2.000000e+00
-; CHECK-NEXT:    [[V1_FR:%.*]] = freeze i1 [[V1]]
-; CHECK-NEXT:    ret i1 [[V1_FR]]
+; CHECK-NEXT:    [[ARG_FR:%.*]] = freeze float [[ARG:%.*]]
+; CHECK-NEXT:    [[V1:%.*]] = fcmp une float [[ARG_FR]], 2.000000e+00
+; CHECK-NEXT:    ret i1 [[V1]]
 ;
   %v1 = fcmp ninf nnan une float %arg, 2.0
   %v1.fr = freeze i1 %v1
   ret i1 %v1.fr
 }
 
 define float @propagate_drop_fmath_select(i1 %arg) {
 ; CHECK-LABEL: @propagate_drop_fmath_select(
-; CHECK-NEXT:    [[V1:%.*]] = select nnan ninf i1 [[ARG:%.*]], float 1.000000e+00, float -1.000000e+00
-; CHECK-NEXT:    [[V1_FR:%.*]] = freeze float [[V1]]
-; CHECK-NEXT:    ret float [[V1_FR]]
+; CHECK-NEXT:    [[ARG_FR:%.*]] = freeze i1 [[ARG:%.*]]
+; CHECK-NEXT:    [[V1:%.*]] = select i1 [[ARG_FR]], float 1.000000e+00, float -1.000000e+00
+; CHECK-NEXT:    ret float [[V1]]
 ;
   %v1 = select ninf nnan i1 %arg, float 1.0, float -1.0
   %v1.fr = freeze float %v1
   ret float %v1.fr
 }
 
 define void @fold_phi_noop(i32 noundef %init, i32 %n) {
 ; CHECK-LABEL: @fold_phi_noop(

llvm/test/Transforms/InstCombine/shuffle_select-inseltpoison.ll

 ; CHECK-NEXT:    ret <4 x float> [[T3]]
 ;
   %t1 = fmul nnan ninf <4 x float> %v0, <float 1.0, float 2.0, float 3.0, float 4.0>
   %t2 = fmul nnan ninf <4 x float> %v0, <float 5.0, float 6.0, float 7.0, float 8.0>
   %t3 = shufflevector <4 x float> %t1, <4 x float> %t2, <4 x i32> <i32 0, i32 5, i32 6, i32 7>
   ret <4 x float> %t3
 }
 
 define <4 x double> @fdiv_fdiv(<4 x double> %v0) {
 ; CHECK-LABEL: @fdiv_fdiv(
-; CHECK-NEXT:    [[T3:%.*]] = fdiv nnan arcp <4 x double> <double undef, double 2.000000e+00, double 7.000000e+00, double 8.000000e+00>, [[V0:%.*]]
+; CHECK-NEXT:    [[T3:%.*]] = fdiv arcp <4 x double> <double undef, double 2.000000e+00, double 7.000000e+00, double 8.000000e+00>, [[V0:%.*]]
 ; CHECK-NEXT:    ret <4 x double> [[T3]]
 ;
   %t1 = fdiv fast <4 x double> <double 1.0, double 2.0, double 3.0, double 4.0>, %v0
   %t2 = fdiv nnan arcp <4 x double> <double 5.0, double 6.0, double 7.0, double 8.0>, %v0
   %t3 = shufflevector <4 x double> %t1, <4 x double> %t2, <4 x i32> <i32 undef, i32 1, i32 6, i32 7>
   ret <4 x double> %t3
 }
 
;
   %t2 = fmul reassoc nsz <4 x float> %v1, <float 5.0, float 6.0, float 7.0, float 8.0>
   %t3 = shufflevector <4 x float> %t1, <4 x float> %t2, <4 x i32> <i32 0, i32 5, i32 6, i32 7>
   ret <4 x float> %t3
 }
 
 define <4 x double> @frem_2_vars(<4 x double> %v0, <4 x double> %v1) {
 ; CHECK-LABEL: @frem_2_vars(
 ; CHECK-NEXT:    [[TMP1:%.*]] = shufflevector <4 x double> [[V0:%.*]], <4 x double> [[V1:%.*]], <4 x i32> <i32 undef, i32 1, i32 6, i32 7>
-; CHECK-NEXT:    [[T3:%.*]] = frem nnan <4 x double> <double undef, double 2.000000e+00, double 7.000000e+00, double 8.000000e+00>, [[TMP1]]
+; CHECK-NEXT:    [[T3:%.*]] = frem <4 x double> <double undef, double 2.000000e+00, double 7.000000e+00, double 8.000000e+00>, [[TMP1]]
 ; CHECK-NEXT:    ret <4 x double> [[T3]]
 ;
   %t1 = frem nnan ninf <4 x double> <double 1.0, double 2.0, double 3.0, double 4.0>, %v0
   %t2 = frem nnan arcp <4 x double> <double 5.0, double 6.0, double 7.0, double 8.0>, %v1
   %t3 = shufflevector <4 x double> %t1, <4 x double> %t2, <4 x i32> <i32 undef, i32 1, i32 6, i32 7>
   ret <4 x double> %t3
 }
 

llvm/test/Transforms/InstCombine/shuffle_select.ll

;
   %t1 = fmul nnan ninf <4 x float> %v0, <float 1.0, float 2.0, float 3.0, float 4.0>
   %t2 = fmul nnan ninf <4 x float> %v0, <float 5.0, float 6.0, float 7.0, float 8.0>
   %t3 = shufflevector <4 x float> %t1, <4 x float> %t2, <4 x i32> <i32 0, i32 5, i32 6, i32 7>
   ret <4 x float> %t3
 }
 
 define <4 x double> @fdiv_fdiv(<4 x double> %v0) {
 ; CHECK-LABEL: @fdiv_fdiv(
-; CHECK-NEXT:    [[T3:%.*]] = fdiv nnan arcp <4 x double> <double undef, double 2.000000e+00, double 7.000000e+00, double 8.000000e+00>, [[V0:%.*]]
+; CHECK-NEXT:    [[T3:%.*]] = fdiv arcp <4 x double> <double undef, double 2.000000e+00, double 7.000000e+00, double 8.000000e+00>, [[V0:%.*]]
 ; CHECK-NEXT:    ret <4 x double> [[T3]]
 ;
   %t1 = fdiv fast <4 x double> <double 1.0, double 2.0, double 3.0, double 4.0>, %v0
   %t2 = fdiv nnan arcp <4 x double> <double 5.0, double 6.0, double 7.0, double 8.0>, %v0
   %t3 = shufflevector <4 x double> %t1, <4 x double> %t2, <4 x i32> <i32 undef, i32 1, i32 6, i32 7>
   ret <4 x double> %t3
 }
 
;
   %t2 = fmul reassoc nsz <4 x float> %v1, <float 5.0, float 6.0, float 7.0, float 8.0>
   %t3 = shufflevector <4 x float> %t1, <4 x float> %t2, <4 x i32> <i32 0, i32 5, i32 6, i32 7>
   ret <4 x float> %t3
 }
 
 define <4 x double> @frem_2_vars(<4 x double> %v0, <4 x double> %v1) {
 ; CHECK-LABEL: @frem_2_vars(
 ; CHECK-NEXT:    [[TMP1:%.*]] = shufflevector <4 x double> [[V0:%.*]], <4 x double> [[V1:%.*]], <4 x i32> <i32 undef, i32 1, i32 6, i32 7>
-; CHECK-NEXT:    [[T3:%.*]] = frem nnan <4 x double> <double undef, double 2.000000e+00, double 7.000000e+00, double 8.000000e+00>, [[TMP1]]
+; CHECK-NEXT:    [[T3:%.*]] = frem <4 x double> <double undef, double 2.000000e+00, double 7.000000e+00, double 8.000000e+00>, [[TMP1]]
 ; CHECK-NEXT:    ret <4 x double> [[T3]]
 ;
   %t1 = frem nnan ninf <4 x double> <double 1.0, double 2.0, double 3.0, double 4.0>, %v0
   %t2 = frem nnan arcp <4 x double> <double 5.0, double 6.0, double 7.0, double 8.0>, %v1
   %t3 = shufflevector <4 x double> %t1, <4 x double> %t2, <4 x i32> <i32 undef, i32 1, i32 6, i32 7>
   ret <4 x double> %t3
 }