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

[Adam Nemet via Phabricator via llvm-commits]

anemet accepted this revision.
anemet added a comment.

LGTM.



================
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())
----------------
paquette wrote:
> 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.
:(

This is how other places seem to be doing:

(Twine("StartLoc") + Twine(i)).str()

which is still probably better since it will only create a single std::string




================
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'
----------------
paquette wrote:
> 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?
Makes sense, thanks for explaining.


https://reviews.llvm.org/D37085