[PATCH] D156767: [AArch64] [BranchRelaxation] Optimize for hot code size in AArch64 branch relaxation

[Mingming Liu via Phabricator via llvm-commits]

mingmingl added a comment.

In D156767#4549141 <https://reviews.llvm.org/D156767#4549141>, @dhoekwater wrote:

> Here's a breakdown of the cases:
>
>   Hot -> Cold [x16 is free across the branch]
>     Do nothing; let the linker relax the branch.
>   
>   Cold -> Hot [x16 is free across the branch]
>     Do nothing; let the linker relax the branch.
>   
>   Hot -> Cold [x16 not free, but others are]
>     Spill x16; let the linker relax the branch.
>   
>   Cold -> Hot [x16 not free, but others are]
>     Manually insert an indirect branch.
>   
>   Hot -> Cold [No free regs]
>     Spill x16; let the linker relax the branch.
>   
>   Cold -> Hot [No free regs]
>     Spill x16 and put the restore block at the end of the hot function; let the linker relax the branch.
>     Ex:
>       [Hot section]
>       func.hot:
>         ... hot code...
>       func.restore:
>         ... restore x16 ...
>         B func.hot
>   
>       [Cold section]
>         func.cold:
>         ... spill x16 ...
>         B func.restore

The change overall looks reasonable to me and the breakdown is well written.

To make the motivation slightly more clearer, maybe describe what's the current status without this patch (hot blocks not optimized, etc), and how this patch helps when cold blocks are split by function-splitting.

In D156767#4549141 <https://reviews.llvm.org/D156767#4549141>, @dhoekwater wrote:

> Here's a breakdown of the cases:
>
>   Hot -> Cold [x16 is free across the branch]
>     Do nothing; let the linker relax the branch.
>   
>   Cold -> Hot [x16 is free across the branch]
>     Do nothing; let the linker relax the branch.
>   
>   Hot -> Cold [x16 not free, but others are]
>     Spill x16; let the linker relax the branch.
>   
>   Cold -> Hot [x16 not free, but others are]
>     Manually insert an indirect branch.
>   
>   Hot -> Cold [No free regs]
>     Spill x16; let the linker relax the branch.
>   
>   Cold -> Hot [No free regs]
>     Spill x16 and put the restore block at the end of the hot function; let the linker relax the branch.
>     Ex:
>       [Hot section]
>       func.hot:
>         ... hot code...
>       func.restore:
>         ... restore x16 ...
>         B func.hot
>   
>       [Cold section]
>         func.cold:
>         ... spill x16 ...
>         B func.restore



================
Comment at: llvm/test/CodeGen/AArch64/branch-relax-b.ll:134
 
+define void @relax_b_x16_taken() {
+; CHECK-LABEL:    relax_b_x16_taken:                      // @relax_b_x16_taken
----------------
nit: add a brief comment about which case (in the breakdown list) is being tested for posterity.

If I read correctly, `bb.3` is the cold block (that would be split out by MFS), and the restore block `.LBB2_4` is the restore block and put at the end of the function. For my information, is `x16` spilled as the 4th or 6th case?


================
Comment at: llvm/test/CodeGen/AArch64/branch-relax-cross-section.mir:30
+  
+  iftrue:                                           ; preds = %entry
+    call void asm sideeffect ".space 4", ""()
----------------
nit: maybe add test cases in an NFC (https://llvm.org/docs/Lexicon.html#n) and manually put cold blocks at the end of the IR function. This way, how the IR function looks like after function-splitting is easier to read, and the diff of prioritized hot code is clearer. 


================
Comment at: llvm/test/CodeGen/AArch64/branch-relax-cross-section.mir:69-71
+    br label %cold
+  
+  cold:                                             ; preds = %entry
----------------
for my information, how does branch-relaxation pass tell `cold:` is a cold block for test case `all_used_cold_to_hot`? It seems `entry` will branch to `cold` unconditionally but I may miss something.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156767