[FPEnv] Last BinaryOperator::isFNeg(...) to m_FNeg(...) changes

[cameron.mcinally]

Summary

Continuing work from D52934 and D53205...

These are the last two BinaryOperator::isFNeg(...) references. Replace them with m_FNeg(...) to avoid regressions when we separate FNeg from the FSub IR instruction.

[spatel]

Nice - that wasn't as bad as I was fearing. :)
I'm not sure how to expose a test diff with the fast-isel change (some combine appears to already be undef aware), but here's an instcombine test:
rL345206

lib/CodeGen/SelectionDAG/FastISel.cpp

1697 ↗

(On Diff #170906)

Use match() here, so we can get rid of getFNegArgument() too?

[cameron.mcinally]

Updating diff based on Sanjay's suggestions. One comment inline...

[cameron.mcinally]

lib/CodeGen/SelectionDAG/FastISel.cpp

1697 ↗

(On Diff #170906)

This would have been a weird use of match(...), i.e. ignoring the return value, so I refactored the code to pull the match(...) into selectFNeg(...). This is also a little weird, but seems ok. Thoughts or alternatives?

[spatel]

lib/CodeGen/SelectionDAG/FastISel.cpp

1697 ↗

(On Diff #170906)

This seems fine to me (definitely better than having 2 match() calls).

lib/Transforms/InstCombine/InstCombineCasts.cpp

1616–1617 ↗

(On Diff #171100)

Did the test that I added fail with this diff?

[cameron.mcinally]

Whoops, forgot to update. Yeah, your vector test looks good now.

[spatel]

LGTM

llvm/trunk/lib/CodeGen/SelectionDAG/FastISel.cpp

 #include "llvm/IR/InstrTypes.h"
 #include "llvm/IR/Instruction.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Mangler.h"
 #include "llvm/IR/Metadata.h"
 #include "llvm/IR/Operator.h"
+#include "llvm/IR/PatternMatch.h"
 #include "llvm/IR/Type.h"
 #include "llvm/IR/User.h"
 #include "llvm/IR/Value.h"
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCInstrDesc.h"
 #include "llvm/MC/MCRegisterInfo.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/MachineValueType.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Target/TargetMachine.h"
 #include "llvm/Target/TargetOptions.h"
 #include <algorithm>
 #include <cassert>
 #include <cstdint>
 #include <iterator>
 #include <utility>
 
 using namespace llvm;
+using namespace PatternMatch;
 
 #define DEBUG_TYPE "isel"
 
 // FIXME: Remove this after the feature has proven reliable.
 static cl::opt<bool> SinkLocalValues("fast-isel-sink-local-values",
                                      cl::init(true), cl::Hidden,
                                      cl::desc("Sink local values in FastISel"));
 
if (FuncInfo.BPI) {
       FuncInfo.MBB->addSuccessorWithoutProb(TrueMBB);
   }
 
   fastEmitBranch(FalseMBB, DbgLoc);
 }
 
 /// Emit an FNeg operation.
 bool FastISel::selectFNeg(const User *I) {
-  unsigned OpReg = getRegForValue(BinaryOperator::getFNegArgument(I));
+  Value *X;
+  if (!match(I, m_FNeg(m_Value(X))))
+    return false;
+  unsigned OpReg = getRegForValue(X);
   if (!OpReg)
     return false;
   bool OpRegIsKill = hasTrivialKill(I);
 
   // If the target has ISD::FNEG, use it.
   EVT VT = TLI.getValueType(DL, I->getType());
   unsigned ResultReg = fastEmit_r(VT.getSimpleVT(), VT.getSimpleVT(), ISD::FNEG,
                                   OpReg, OpRegIsKill);

 bool FastISel::selectOperator(const User *I, unsigned Opcode) {
   switch (Opcode) {
   case Instruction::Add:
     return selectBinaryOp(I, ISD::ADD);
   case Instruction::FAdd:
     return selectBinaryOp(I, ISD::FADD);
   case Instruction::Sub:
     return selectBinaryOp(I, ISD::SUB);
-  case Instruction::FSub:
+  case Instruction::FSub: 
     // FNeg is currently represented in LLVM IR as a special case of FSub.
-    if (BinaryOperator::isFNeg(I))
-      return selectFNeg(I);
-    return selectBinaryOp(I, ISD::FSUB);
+    return selectFNeg(I) || selectBinaryOp(I, ISD::FSUB);
   case Instruction::Mul:
     return selectBinaryOp(I, ISD::MUL);
   case Instruction::FMul:
     return selectBinaryOp(I, ISD::FMUL);
   case Instruction::SDiv:
     return selectBinaryOp(I, ISD::SDIV);
   case Instruction::UDiv:
     return selectBinaryOp(I, ISD::UDIV);

llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp

switch (OpI->getOpcode()) {
         }
 
         Value *ExactResult = Builder.CreateFRemFMF(LHS, RHS, OpI);
         return CastInst::CreateFPCast(ExactResult, Ty);
       }
     }
 
     // (fptrunc (fneg x)) -> (fneg (fptrunc x))
-    if (BinaryOperator::isFNeg(OpI)) {
-      Value *InnerTrunc = Builder.CreateFPTrunc(OpI->getOperand(1), Ty);
+    Value *X;
+    if (match(OpI, m_FNeg(m_Value(X)))) {
+      Value *InnerTrunc = Builder.CreateFPTrunc(X, Ty);
       return BinaryOperator::CreateFNegFMF(InnerTrunc, OpI);
     }
   }
 
   if (auto *II = dyn_cast<IntrinsicInst>(FPT.getOperand(0))) {
     switch (II->getIntrinsicID()) {
     default: break;
     case Intrinsic::ceil:

llvm/trunk/test/Transforms/InstCombine/fpcast.ll

 ;
   %b = fsub float -0.0, %a
   %c = fptrunc float %b to half
   ret half %c
 }
 
 define <2 x half> @fneg_fptrunc_vec_undef(<2 x float> %a) {
 ; CHECK-LABEL: @fneg_fptrunc_vec_undef(
-; CHECK-NEXT:    [[B:%.*]] = fsub <2 x float> <float -0.000000e+00, float undef>, [[A:%.*]]
-; CHECK-NEXT:    [[C:%.*]] = fptrunc <2 x float> [[B]] to <2 x half>
+; CHECK-NEXT:    [[TMP1:%.*]] = fptrunc <2 x float> [[A:%.*]] to <2 x half>
+; CHECK-NEXT:    [[C:%.*]] = fsub <2 x half> <half 0xH8000, half 0xH8000>, [[TMP1]]
 ; CHECK-NEXT:    ret <2 x half> [[C]]
 ;
   %b = fsub <2 x float> <float -0.0, float undef>, %a
   %c = fptrunc <2 x float> %b to <2 x half>
   ret <2 x half> %c
 }
 
 define half @test4-fast(float %a) {