[llvm][nfc] InstructionCostDetail encapsulation

[mtrofin]

Summary

Ensured initialized fields; encapsulad delta calulations and evaluation of threshold having had changed; assertion for CostThresholdMap dereference, to capture design intent.

[knaumov]

LGTM

[davidxl]

lgtm

[mtrofin]

b4924f01a45c03f4f1e7a504e3ba917a354c1ef9

[apilipenko]

llvm/lib/Analysis/InlineCost.cpp

731–732

It's possible that we don't analyze all instructions. The analysis can stop early if we exceed the threshold. The original code had a problem with this scenario as well.

We just need to have an early exit.

[mtrofin]

llvm/lib/Analysis/InlineCost.cpp

731–732

Can you share the repro, so we can add a regression test as well?

llvm/lib/Analysis/InlineCost.cpp

cl::desc("Allow inlining when caller has a superset of callee's nobuiltin "
              "attributes."));
 
 namespace {
 class InlineCostCallAnalyzer;
 
 // This struct is used to store information about inline cost of a
 // particular instruction
 struct InstructionCostDetail {
-  int CostBefore;
-  int CostAfter;
-  int ThresholdBefore;
-  int ThresholdAfter;
+  int CostBefore = 0;
+  int CostAfter = 0;
+  int ThresholdBefore = 0;
+  int ThresholdAfter = 0;
+
+  int getThresholdDelta() const { return ThresholdAfter - ThresholdBefore; }
+
+  int getCostDelta() const { return CostAfter - CostBefore; }
+
+  bool hasThresholdChanged() const { return ThresholdAfter != ThresholdBefore; }
 };
 
 class CostAnnotationWriter : public AssemblyAnnotationWriter {
 public:
   // This DenseMap stores the delta change in cost and threshold after
   // accounting for the given instruction.
   DenseMap <const Instruction *, InstructionCostDetail> CostThresholdMap;
 
void CallAnalyzer::disableSROAForArg(AllocaInst *SROAArg) {
   disableLoadElimination();
 }
 
 void CostAnnotationWriter::emitInstructionAnnot(
     const Instruction *I, formatted_raw_ostream &OS) {
     // The cost of inlining of the given instruction is printed always.
     // The threshold delta is printed only when it is non-zero. It happens
     // when we decided to give a bonus at a particular instruction.
-    OS << "; cost before = " << CostThresholdMap[I].CostBefore <<
-              ", cost after = " << CostThresholdMap[I].CostAfter <<
-              ", threshold before = " << CostThresholdMap[I].ThresholdBefore <<
-              ", threshold after = " << CostThresholdMap[I].ThresholdAfter <<
-              ", ";
-    OS << "cost delta = " << CostThresholdMap[I].CostAfter -
-                                CostThresholdMap[I].CostBefore;
-    if (CostThresholdMap[I].ThresholdAfter != CostThresholdMap[I].ThresholdBefore)
-      OS << ", threshold delta = " << CostThresholdMap[I].ThresholdAfter -
-                                CostThresholdMap[I].ThresholdBefore;
+    assert(CostThresholdMap.count(I) > 0 &&
+           "Expected each instruction to have an instruction annotation");
+    const auto &Record = CostThresholdMap[I];
+    OS << "; cost before = " << Record.CostBefore
+       << ", cost after = " << Record.CostAfter
+       << ", threshold before = " << Record.ThresholdBefore
+       << ", threshold after = " << Record.ThresholdAfter << ", ";
+    OS << "cost delta = " << Record.getCostDelta();
+    if (Record.hasThresholdChanged())
+      OS << ", threshold delta = " << Record.getThresholdDelta();
     OS << "\n";
 }
 
 /// If 'V' maps to a SROA candidate, disable SROA for it.
 void CallAnalyzer::disableSROA(Value *V) {
   if (auto *SROAArg = getSROAArgForValueOrNull(V)) {
     disableSROAForArg(SROAArg);
   }