[PATCH] D70104: [BPI] Improve unreachable/ColdCall heurstics to handle loops.

[Serguei Katkov via Phabricator via llvm-commits]

skatkov added a comment.

The implementation itself looks good to me.
I worry about tests whether fixes of the tests are expected.
If you have ready answers please let me know otherwise I'll try to dig into them deeply.



================
Comment at: llvm/lib/Analysis/BranchProbabilityInfo.cpp:159
+    if (TargetSet.insert(BB).second)
+      NewItems.insert(pred_begin(BB), pred_end(BB));
+  WorkList.insert(WorkList.end(), NewItems.begin(), NewItems.end());
----------------
Theoretically predecessor might be already marked in TargetSet so you could potentially filter the adding list of predecessors.


================
Comment at: llvm/test/CodeGen/X86/pr37916.ll:10
 ; CHECK:       # %bb.0: # %entry
-; CHECK-NEXT:    .p2align 4, 0x90
 ; CHECK-NEXT:  .LBB0_1: # %if.end
----------------
Why we lost this alignment?


================
Comment at: llvm/test/CodeGen/X86/ragreedy-hoist-spill.ll:5
 ; This testing case is reduced from 254.gap SyFgets function.
 ; We make sure a spill is hoisted to a cold BB inside the hotter outer loop.
 
----------------
I have a trouble to detect whether the expected behavior of the test is still performed.

Did you ensured in it or just re-generated the expected output?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70104/new/

https://reviews.llvm.org/D70104