[llvm] [LoopPeel] Fix branch weights' effect on block frequencies (PR #128785)

[Mircea Trofin via llvm-commits]

================
@@ -850,27 +852,35 @@ llvm::getLoopEstimatedTripCount(Loop *L,
             getEstimatedTripCount(LatchBranch, L, ExitWeight)) {
       if (EstimatedLoopInvocationWeight)
         *EstimatedLoopInvocationWeight = ExitWeight;
+      if (auto EstimatedTripCount =
----------------
mtrofin wrote:

> Currently in this PR, the new metadata is not introduced until a pass like LoopPeel calls setLoopEstimatedTripCount. Normally that would be because it creates the situation where branch weights alone cannot encode both block frequencies and estimated trip counts with desired values, so the new metadata is then required (but of course not all passes have been fixed yet to maintain block frequencies correctly).

Could this cause a surprise if other loop transforms come first, i.e. should the metadata be inserted earlier, maybe by a small pass dedicated to this job?

> Moreover, the todo comment this PR introduces into LoopPeel.cpp describes how getLoopEstimatedTripCount and setLoopEstimatedTripCount cannot handle some loop forms and so don't get/set the estimated trip count. That issue exists without this PR. What's new is that it skips adding the new metadata in that case.

Ack. How about inserting a form of this metadata that says "unknown". So then we can at least distinguish between "pass dropped the info == bug" from "info wasn't dropped, but you can't use it". Has the same effect wrt the trip count calculation, but (my argument goes) it's way more maintainable (I'd like to do the same for `MD_prof` fwiw - i.e. "unknown" vs absence)

> As we discussed at some point in the RFC, the inconsistent presence of the new metadata is not ideal, and we might want to ultimately change that. But in the long term, it sounds like people might want to drop estimated trip counts altogether, so I'm not yet convinced that efforts to make the new metadata more consistently present are worthwhile. What do you think?

I think my suggestion is minimal: adding the "unknown" form of the metadata and then checking and (at minimum) LLVM_DEBUG - ing when a loop doesn't have it whatsoever. I get the concern about investment vs risk of future (but currently unknown, IIUC) alternatives. I think making it hard for passes to naively drop it would help its value a lot.

https://github.com/llvm/llvm-project/pull/128785