[SCCP] Add UnaryOperator visitor to SCCP

[cameron.mcinally]

Summary

I don't know this pass at all, so would appreciate a thorough review.

I did notice that FP BinaryOperators are handled conservatively. FNeg is more an XOR than an FP operation, so I think I handled it appropriately. Please advise...

[craig.topper]

llvm/lib/Transforms/Scalar/SCCP.cpp

996

Binary->Unary

[cameron.mcinally]

Fix comment: Binary->Unary.

[fhahn]

Thanks for adding this.

Please also add non-undef tests for unary operators, e.g. something like '%v = fneg double 10.0` or %v1 = fadd double 10.0, 20.0; %v2 = fneg double %v1.

llvm/lib/Transforms/Scalar/SCCP.cpp

975

nit: I think Op0V or something that makes it explicit that this ties back to the first operand of the instruction would be a slightly better name.

[cameron.mcinally]

Rename VState to V0State as requested.

Also add constant folding test for unary fneg.

[cameron.mcinally]

llvm/lib/Transforms/Scalar/SCCP.cpp

975

Oh, just realized you requested Op0V instead of V0State. I copied that naming from visitBinaryOperator(...). Are you okay with this?

[fhahn]

llvm/test/Transforms/SCCP/apfloat-basictest.ll

5

I think it would be better to explicitly check the return value, instead of relying on grep. Also, would it be possible to add another case where the operand to fneg itself needs to be propagated before we simplify fneg?

[cameron.mcinally]

Add a test as suggested...

[fhahn]

LGTM, thanks!

[fhahn]

llvm/test/Transforms/SCCP/apfloat-basictest.ll

5

Needs FileCheck now.

[cameron.mcinally]

Add FileCheck...

llvm/lib/Transforms/Scalar/SCCP.cpp

private:
 
   // Terminators
 
   void visitReturnInst(ReturnInst &I);
   void visitTerminator(Instruction &TI);
 
   void visitCastInst(CastInst &I);
   void visitSelectInst(SelectInst &I);
+  void visitUnaryOperator(Instruction &I);
   void visitBinaryOperator(Instruction &I);
   void visitCmpInst(CmpInst &I);
   void visitExtractValueInst(ExtractValueInst &EVI);
   void visitInsertValueInst(InsertValueInst &IVI);
 
   void visitCatchSwitchInst(CatchSwitchInst &CPI) {
     markOverdefined(&CPI);
     visitTerminator(CPI);
void SCCPSolver::visitSelectInst(SelectInst &I) {
 
   if (TVal.isUnknown())   // select ?, undef, X -> X.
     return (void)mergeInValue(&I, FVal);
   if (FVal.isUnknown())   // select ?, X, undef -> X.
     return (void)mergeInValue(&I, TVal);
   markOverdefined(&I);
 }
 
+// Handle Unary Operators.
+void SCCPSolver::visitUnaryOperator(Instruction &I) {
+  LatticeVal V0State = getValueState(I.getOperand(0));
+
+  LatticeVal &IV = ValueState[&I];
+  if (IV.isOverdefined()) return;
+
+  if (V0State.isConstant()) {
+    Constant *C = ConstantExpr::get(I.getOpcode(), V0State.getConstant());
+
+    // op Y -> undef.
+    if (isa<UndefValue>(C))
+      return;
+    return (void)markConstant(IV, &I, C);
+  }
+
+  // If something is undef, wait for it to resolve.
+  if (!V0State.isOverdefined())
+    return;
+
+  markOverdefined(&I);
+}
+
 // Handle Binary Operators.
 void SCCPSolver::visitBinaryOperator(Instruction &I) {
   LatticeVal V1State = getValueState(I.getOperand(0));
   LatticeVal V2State = getValueState(I.getOperand(1));
 
   LatticeVal &IV = ValueState[&I];
   if (IV.isOverdefined()) return;
 
   if (V1State.isConstant() && V2State.isConstant()) {
for (Instruction &I : BB) {
       case Instruction::FDiv:
       case Instruction::FRem:
         // Floating-point binary operation: be conservative.
         if (Op0LV.isUnknown() && Op1LV.isUnknown())
           markForcedConstant(&I, Constant::getNullValue(ITy));
         else
           markOverdefined(&I);
         return true;
+      case Instruction::FNeg:
+        break; // fneg undef -> undef
       case Instruction::ZExt:
       case Instruction::SExt:
       case Instruction::FPToUI:
       case Instruction::FPToSI:
       case Instruction::FPExt:
       case Instruction::PtrToInt:
       case Instruction::IntToPtr:
       case Instruction::SIToFP:

llvm/test/Transforms/SCCP/apfloat-basictest.ll

+; This is a basic sanity check for constant propagation. The fneg instruction
+; should be eliminated.
+
+; RUN: opt < %s -sccp -S | FileCheck %s
+
+define double @test(i1 %B) {
+  br i1 %B, label %BB1, label %BB2
+BB1:
+  %Val = fneg double 42.0
+  br label %BB3
+BB2:
+  br label %BB3
+BB3:
+  %Ret = phi double [%Val, %BB1], [1.0, %BB2]
+  ret double %Ret
+; CHECK-LABEL: @test(
+; CHECK: [[PHI:%.*]] = phi double [ -4.200000e+01, %BB1 ], [ 1.000000e+00, %BB2 ]
+}
+
+define double @test1(i1 %B) {
+        br i1 %B, label %BB1, label %BB2
+BB1:    
+        %Div = fdiv double 1.0, 1.0        
+        %Val = fneg double %Div
+        br label %BB3
+BB2:
+        br label %BB3
+BB3:
+        %Ret = phi double [%Val, %BB1], [1.0, %BB2]
+        ret double %Ret
+; CHECK-LABEL: @test1(
+; CHECK: [[PHI:%.*]] = phi double [ -1.000000e+00, %BB1 ], [ 1.000000e+00, %BB2 ]
+}

llvm/test/Transforms/SCCP/undef-resolve.ll

 
 define i32 @test11(i1 %tobool) {
 entry:
   %shr4 = ashr i32 undef, zext (i1 icmp eq (i32* bitcast (i32 (i1)* @test11 to i32*), i32* @GV) to i32)
   ret i32 %shr4
 ; CHECK-LABEL: @test11(
 ; CHECK: ret i32 0
 }
+
+; Test unary ops
+define double @test12(double %x) {
+  %t = fneg double undef
+  ret double %t
+; CHECK-LABEL: @test12(
+; CHECK: double undef
+}