[InlineCost] Add support for unary fneg.

[craig.topper]

Summary

This adds support for unary fneg based on the implementation of BinaryOperator without the soft float FP cost.

Previously we would just delegate to visitUnaryInstruction. I think the only real change is that we will pass the FastMath flags to SimplifyFNeg now.

Unfortunately, SimplifyFNegInst does not currently do anything with the fastmath flags so I don't think this changes any behavior. Thus I don't know how to test it.

[efriedma]

llvm/lib/Analysis/InlineCost.cpp

1134

Is this actually true? fneg should be one or two native instructions for almost every combination of type/target, even ones that don't have native floating-point support.

[craig.topper]

llvm/lib/Analysis/InlineCost.cpp

1134

Probably not. But it is the answer we would have gotten for fsub -0.0, %x. Should we exclude both forms of writing fneg from this?

[cameron.mcinally]

LGTM.

I'm not that familiar with InlineCost, but I see that almost all of this is copied from visitBinaryOperator(...), so I'm pretty confident in the review.

I was able to test the increased fp cost. The included test case will report a cost of 100 before this patch and 125 after this patch. I based this on an earlier arm test case in the test file and just changed an fmul by constant to an fneg. I can pre-commit this test but wanted opinions on if this test needs changes.

FMul seems like a good first attempt. Maybe a cost of XOR by constant is better, but they're probably pretty similar anyway.

[cameron.mcinally]

llvm/lib/Analysis/InlineCost.cpp

1134

This seems like a good check to have around for a general UnaryOperator. Of course there's only one right now, but who knows in the future...

[efriedma]

FMul seems like a good first attempt. Maybe a cost of XOR by constant is better, but they're probably pretty similar anyway.

We're talking about the difference in cost between a runtime call and a xor by a constant; those are not really similar.

llvm/lib/Analysis/InlineCost.cpp

1134

We probably should treat "fsub -0.0, %x" the same way, yes.

It probably doesn't makes sense to try to predict the costs of future unary operators that don't current exist.

[cameron.mcinally]

In D62699#1524170, @efriedma wrote:

FMul seems like a good first attempt. Maybe a cost of XOR by constant is better, but they're probably pretty similar anyway.

We're talking about the difference in cost between a runtime call and a xor by a constant; those are not really similar.

Those are different. Why is the cost calculation so pessimistic? For soft float targets?

[efriedma]

Yes, soft-float targets, since TTI.getFPOpCost(I.getType()) == TargetTransformInfo::TCC_Expensive is true.

[craig.topper]

Restrict to just FNeg since we have no other UnaryOperators yet.

[cameron.mcinally]

This is a good compromise. I'm okay with this if Eli is...

[efriedma]

It should be possible to write a testcase based on the simplification? Otherwise looks fine.

[craig.topper]

The only optimizations that are in simplifyFNegInst currently are constant folding and (fneg (fneg X))->X. The constant folding case would already have been supported by using visitUnaryInstruction. For the (fneg (fneg X)) case, will InlineCost be able to do something if one fneg is in the caller and the other is in the callee?

[efriedma]

Is it likely that we're ever going to be able to do anything useful with the fast-math flags?

Even if the new code behaves the same way as visitUnaryInstruction, I'd like to have some test coverage, to show this doesn't break anything.

[cameron.mcinally]

In D62699#1528428, @efriedma wrote:

Is it likely that we're ever going to be able to do anything useful with the fast-math flags?

I don't think that decision has been made. It's just a bit-twiddle, so I'm not sure if FMFs would give us anything.

That said, if we knew nnan and nsz, we could convert a unary FNeg into a binary FNeg. I don't think that would be a win, but haven't studied it...

[craig.topper]

Add test case that should requires constant folding an fneg.

[efriedma]

LGTM

llvm/lib/Analysis/InlineCost.cpp

class CallAnalyzer : public InstVisitor<CallAnalyzer, bool> {
   bool visitBitCast(BitCastInst &I);
   bool visitPtrToInt(PtrToIntInst &I);
   bool visitIntToPtr(IntToPtrInst &I);
   bool visitCastInst(CastInst &I);
   bool visitUnaryInstruction(UnaryInstruction &I);
   bool visitCmpInst(CmpInst &I);
   bool visitSub(BinaryOperator &I);
   bool visitBinaryOperator(BinaryOperator &I);
+  bool visitFNeg(UnaryOperator &I);
   bool visitLoad(LoadInst &I);
   bool visitStore(StoreInst &I);
   bool visitExtractValue(ExtractValueInst &I);
   bool visitInsertValue(InsertValueInst &I);
   bool visitCallBase(CallBase &Call);
   bool visitReturnInst(ReturnInst &RI);
   bool visitBranchInst(BranchInst &BI);
   bool visitSelectInst(SelectInst &SI);
bool CallAnalyzer::visitBinaryOperator(BinaryOperator &I) {
   if (I.getType()->isFloatingPointTy() &&
       TTI.getFPOpCost(I.getType()) == TargetTransformInfo::TCC_Expensive &&
       !match(&I, m_FNeg(m_Value())))
     addCost(InlineConstants::CallPenalty);
 
   return false;
 }
 
+bool CallAnalyzer::visitFNeg(UnaryOperator &I) {
+  Value *Op = I.getOperand(0);
+  Constant *COp = dyn_cast<Constant>(Op);
+  if (!COp)
+    COp = SimplifiedValues.lookup(Op);
+
+  Value *SimpleV = SimplifyFNegInst(COp ? COp : Op,
+                                    cast<FPMathOperator>(I).getFastMathFlags(),
+                                    DL);
+
+  if (Constant *C = dyn_cast_or_null<Constant>(SimpleV))
+    SimplifiedValues[&I] = C;
+
+  if (SimpleV)
+    return true;
+
+  // Disable any SROA on arguments to arbitrary, unsimplified fneg.
+  disableSROA(Op);
+
+  return false;
+}
+
 bool CallAnalyzer::visitLoad(LoadInst &I) {
   Value *SROAArg;
   DenseMap<Value *, int>::iterator CostIt;
   if (lookupSROAArgAndCost(I.getPointerOperand(), SROAArg, CostIt)) {
     if (I.isSimple()) {
       accumulateSROACost(CostIt, InlineConstants::InstrCost);
       return true;
     }
 
     disableSROA(CostIt);
   }

llvm/test/Transforms/Inline/inline_constprop.ll

 bb1:
   %call = call i16 @caller7.external(i16 1)
   call void @callee7(i16 0, i16 %call)
   ret void
 }
 ; CHECK-LABEL: define void @caller7(
 ; CHECK: %call = call i16 @caller7.external(i16 1)
 ; CHECK-NEXT: ret void
+
+define float @caller8(float %y) {
+; Check that we can constant-prop through fneg instructions
+;
+; CHECK-LABEL: @caller8(
+; CHECK-NOT: call
+; CHECK: ret
+  %x = call float @callee8(float -42.0, float %y)
+  ret float %x
+}
+
+define float @callee8(float %x, float %y) {
+  %neg = fneg float %x
+  %icmp = fcmp ugt float %neg, 42.0
+  br i1 %icmp, label %bb.true, label %bb.false
+
+bb.true:
+  ; This block musn't be counted in the inline cost.
+  %y1 = fadd float %y, 1.0
+  %y2 = fadd float %y1, 1.0
+  %y3 = fadd float %y2, 1.0
+  %y4 = fadd float %y3, 1.0
+  %y5 = fadd float %y4, 1.0
+  %y6 = fadd float %y5, 1.0
+  %y7 = fadd float %y6, 1.0
+  %y8 = fadd float %y7, 1.0
+  ret float %y8
+
+bb.false:
+  ret float %x
+}