[PATCH] D30631: [BPI] Use metadata info before any other heuristics

[Serguei Katkov via Phabricator via llvm-commits]

skatkov marked 2 inline comments as done.
skatkov added inline comments.


================
Comment at: lib/Analysis/BranchProbabilityInfo.cpp:308-311
+      auto MetadataProb = BranchProbability::getBranchProbability(
+          Weights[i], static_cast<uint32_t>(WeightSum));
+      if (UnreachableProb < MetadataProb)
+        return false;
----------------
chandlerc wrote:
> I feel like it would be nicer to just adjust the weight downward such that the probability is essentially the minimum of the two sources of information. That way we don't lose the metadata's weights for the different successors that *don't* go to unreachable.
> 
> Consider the test case (in pseudo C code):
> 
>   for (...) {
>     switch (cond) {
>       default:
>         unreachable
>       case 2: // HOT
>         // something tiny
>         continue;
>       case 3: // COLD
>         // huge pile of ugly code
>         continue;
>       }
>     }
>   }
> 
> If, for whatever reason, we end up with one sample in the metadata going to unreachable, we'll completely loose the metadata that distinguishes between hot and cold here.
> 
> Does that make sense?
It is possible, however I try to follow simpler logic here. So the main question if we do not trust that metadata represents the value for unreachable edge correctly (we fix it by the weight downward) why we trust that data for hot/cold edges is valid and continue using it?

However if you still insist on that I would propose I will create a follow up patch implementing this approach and leave this patch as is. Is it ok for you?




https://reviews.llvm.org/D30631