[EarlyCSE] Add support for unary FNeg to EarlyCSE

[cameron.mcinally]

Remove unnecessary {}'s.

[cameron.mcinally]

Ah, I misunderstood hash_combine(...). This patch needs work...

[cameron.mcinally]

Correct hash_combine(...) use.

[JosephTremoulet]

llvm/lib/Transforms/Scalar/EarlyCSE.cpp

168–169

I think this is the same thing you'd get by letting control fall down to the default case at the end of the function (once you relax the assertion there to mirror the change you've made to canHandle). I'd think that would be preferable as it would keep getHashValueImpl and isEqualImpl more directly/syntactically/obviously in sync.

[cameron.mcinally]

Updated patch for Joe's suggestion.

[JosephTremoulet]

LGTM. Probably good to have someone else weigh in before committing (I touched this code last, but only to fix a couple bugs).

[spatel]

LGTM

llvm/lib/Transforms/Scalar/EarlyCSE.cpp

bool isSentinel() const {
     return Inst == DenseMapInfo<Instruction *>::getEmptyKey() ||
            Inst == DenseMapInfo<Instruction *>::getTombstoneKey();
   }
 
   static bool canHandle(Instruction *Inst) {
     // This can only handle non-void readnone functions.
     if (CallInst *CI = dyn_cast<CallInst>(Inst))
       return CI->doesNotAccessMemory() && !CI->getType()->isVoidTy();
-    return isa<CastInst>(Inst) || isa<BinaryOperator>(Inst) ||
-           isa<GetElementPtrInst>(Inst) || isa<CmpInst>(Inst) ||
-           isa<SelectInst>(Inst) || isa<ExtractElementInst>(Inst) ||
-           isa<InsertElementInst>(Inst) || isa<ShuffleVectorInst>(Inst) ||
-           isa<ExtractValueInst>(Inst) || isa<InsertValueInst>(Inst);
+    return isa<CastInst>(Inst) || isa<UnaryOperator>(Inst) ||
+           isa<BinaryOperator>(Inst) || isa<GetElementPtrInst>(Inst) ||
+           isa<CmpInst>(Inst) || isa<SelectInst>(Inst) ||
+           isa<ExtractElementInst>(Inst) || isa<InsertElementInst>(Inst) ||
+           isa<ShuffleVectorInst>(Inst) || isa<ExtractValueInst>(Inst) ||
+           isa<InsertValueInst>(Inst);
   }
 };
 
 } // end anonymous namespace
 
 namespace llvm {
 
 template <> struct DenseMapInfo<SimpleValue> {
static bool matchSelectWithOptionalNotCond(Value *V, Value *&Cond, Value *&A,
   else
     Flavor = SPF_UNKNOWN;
 
   return true;
 }
 
 static unsigned getHashValueImpl(SimpleValue Val) {
   Instruction *Inst = Val.Inst;
+
+  if (UnaryOperator *UnOp = dyn_cast<UnaryOperator>(Inst))
+    return hash_combine(UnOp->getOpcode(), UnOp->getOperand(0));
+
   // Hash in all of the operands as pointers.
   if (BinaryOperator *BinOp = dyn_cast<BinaryOperator>(Inst)) {
     Value *LHS = BinOp->getOperand(0);
     Value *RHS = BinOp->getOperand(1);
     if (BinOp->isCommutative() && BinOp->getOperand(0) > BinOp->getOperand(1))
       std::swap(LHS, RHS);
 
     return hash_combine(BinOp->getOpcode(), LHS, RHS);

llvm/test/Transforms/EarlyCSE/floatingpoint.ll

define <4 x float> @fW( <4 x float> %a) {
        ; CHECK: ret <4 x float> %a
        %b = fadd  <4 x float> %a, <float -0.0,float -0.0,float -0.0,float -0.0>
        ret <4 x float> %b
 }
 
 ; CSE unary fnegs.
 define void @fX(<4 x float> *%p, <4 x float> %a) {
        ; CHECK: %x = fneg <4 x float> %a
-       ; CHECK: %y = fneg <4 x float> %a
-       ; CHECK-NEXT:  store volatile <4 x float> %x, <4 x float>* %p
-       ; CHECK-NEXT: store volatile <4 x float> %y, <4 x float>* %p
+       ; CHECK-NEXT: store volatile <4 x float> %x, <4 x float>* %p
+       ; CHECK-NEXT: store volatile <4 x float> %x, <4 x float>* %p
        %x = fneg <4 x float> %a
        %y = fneg <4 x float> %a
        store volatile <4 x float> %x, <4 x float>* %p
        store volatile <4 x float> %y, <4 x float>* %p
        ret void
 }