[PATCH] D129178: [RISCV] Enable the GlobalMerge pass by default

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.
Herald added a subscriber: luke.

The following is basically a brain dump on a few things vaguely related to GlobalMerge for RISCV.  This isn't a review comment on this review per se.  Some of this came from discussion w/Palmer because I nerd sniped myself into thinking this a bit too hard, and he was willing to brainstorm with me.  .

Profitability wise, we could potentially restrict GM to cases where the alignment guarantees the second address could fold into the consuming load/store instruction.   The simplest case would be to restrict to when at least one of the globals being merged had a sufficiently large alignment.  https://reviews.llvm.org/D129686#inline-1380320 has some brainstorming on a more advanced boundary align mechanism, but building that out is likely non trivial.  There have been some other use cases for analogous features in the past, but I don't have details.

For the GP interaction, we may want to take a close look at how gcc models global merging vs how we do.  Per Palmer, it keeps around the symbols for each global, and that may impact the heuristic that LD uses for selecting globals to place near GP.  We may be able to massage our output a bit to line up with the existing heuristics.

There's a question of how worthwhile this is.   For anything beyond static builds with medlow, we need to worry about pc relative addresses.  Given that, we'd need to additionally account for the alignment of the auipc.  We could potentially insert an align directive, but that wastes space.  Per Palmer, there was some previous discussion around a relocation type for an optimized "aligned auipc" construct which used (at most) a single extra instruction.  However, no one has pushed this forward.

My current thinking is that we should probably enable this for code size minimization only, and return to it at a later point.


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

https://reviews.llvm.org/D129178