[PATCH] D37085: [MachineOutliner] Add missed optimization remarks based off outliner cost model

[Jessica Paquette via Phabricator via llvm-commits]

paquette marked 3 inline comments as done.
paquette added inline comments.


================
Comment at: lib/CodeGen/MachineOutliner.cpp:931
+      for (size_t e = CandidateClass.size() - 1; i < e; i++) {
+        R << NV("StartLoc" + std::to_string(i),
+                CandidateClass[i].first->getDebugLoc())
----------------
anemet wrote:
> Does this work: "StartLoc" + Twine(i)
> 
> I would call this OtherStartLoc<i> for easier differentiation form the main loc.
Twines don't seem to work because apparently there's no conversion to a StringRef? I guess that's something that we should probably add, unless I'm doing something wrong.


================
Comment at: test/CodeGen/AArch64/machine-outliner-remarks.ll:11-18
+; YAML-NEXT:   - String:          ': Did not outline '
+; YAML-NEXT:   - Length:          '2'
+; YAML-NEXT:   - String:          ' instructions. Outlined instruction count ('
+; YAML-NEXT:   - OutliningCost:   '9'
+; YAML-NEXT:   - String:          ')'
+; YAML-NEXT:   - String:          ' >= unoutlined instruction count ('
+; YAML-NEXT:   - NotOutliningCost: '4'
----------------
anemet wrote:
> I am not sure I actually understand diagnostics here.  Shouldn't the non-outlined count (4) be the same as the number of instructions (2)?
NotOutliningCost is the sum of the lengths of each candidate related to a repeated sequence of instructions. In this case, the sequence is length 2 with 2 occurrences, so we have 2 + 2 = 4.

Similarly, OutliningCost is the sum of the number of instructions taken to call a theoretical outlined function made from some sequence, plus the length of the sequence and anything else we'd have to add to create the frame for the function. In this case, we'd insert 3 instructions for each call, 2 instructions for the sequence, plus 1 for the return. So, we get 9.

Maybe this is somewhat unclear? A name like TotalOutlinedInstructionCount/TotalNotOutlinedInstructionCount might be better?


https://reviews.llvm.org/D37085