PHI nodes are not `FPMathOperator` s

[arsenm]

Test case?

[sanjoy]

In D58887#1416602, @arsenm wrote:

Test case?

I need this in a dependent change which has a test that fails without this fix. I can also add a C++ unit test in this change if you think that will help (I don't see an obvious way to test this using llvm-lit).

[arsenm]

I think unit tests are good for this sort of thing

[sanjoy]

Add unit test

[arsenm]

LGTM

[sanjoy]

Matt, I noticed a necessary change I had to make before landing this separately.
I didn't notice this because I was initially testing this combined with some
other changes.

include/llvm/IR/Operator.h

static bool classof(const Value *V) {
 
     switch (Opcode) {
     case Instruction::FCmp:
       return true;
     // non math FP Operators (no FMF)
     case Instruction::ExtractElement:
     case Instruction::ShuffleVector:
     case Instruction::InsertElement:
+    case Instruction::PHI:
       return false;
     default:
       return V->getType()->isFPOrFPVectorTy();
     }
   }
 };
 
 /// A helper template for defining operators for individual opcodes.

lib/Analysis/IVDescriptors.cpp

if (m_FMul(m_Value(Op1), m_Value(Op2)).match(I1) && (I1->isFast()))
     return InstDesc(Kind == RK_FloatMult, SI);
 
   return InstDesc(false, I);
 }
 
 RecurrenceDescriptor::InstDesc
 RecurrenceDescriptor::isRecurrenceInstr(Instruction *I, RecurrenceKind Kind,
                                         InstDesc &Prev, bool HasFunNoNaNAttr) {
-  bool FP = I->getType()->isFloatingPointTy();
   Instruction *UAI = Prev.getUnsafeAlgebraInst();
-  if (!UAI && FP && !I->isFast())
+  if (!UAI && isa<FPMathOperator>(I) && !I->isFast())
     UAI = I; // Found an unsafe (unvectorizable) algebra instruction.
 
   switch (I->getOpcode()) {
   default:
     return InstDesc(false, I);
   case Instruction::PHI:
     return InstDesc(I, Prev.getMinMaxKind(), Prev.getUnsafeAlgebraInst());
   case Instruction::Sub:

unittests/IR/InstructionsTest.cpp

TEST(InstructionsTest, SkipDebug) {
   auto *Term = BB.getTerminator();
   EXPECT_EQ(Term, BB.begin()->getNextNonDebugInstruction());
   EXPECT_EQ(Term->getIterator(), skipDebugIntrinsics(BB.begin()));
 
   // After the terminator, there are no non-debug instructions.
   EXPECT_EQ(nullptr, Term->getNextNonDebugInstruction());
 }
 
+TEST(InstructionsTest, PhiIsNotFPMathOperator) {
+  LLVMContext Context;
+  IRBuilder<> Builder(Context);
+  MDBuilder MDHelper(Context);
+  Instruction *I = Builder.CreatePHI(Builder.getDoubleTy(), 0);
+  EXPECT_FALSE(isa<FPMathOperator>(I));
+  I->deleteValue();
+}
+
 } // end anonymous namespace
 } // end namespace llvm