[PATCH] D150009: [AArch64CompressJumpTables] Prevent over-compression when block alignment is bigger than function alignment.

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

I'm pretty sure what branch relaxation is doing is wrong, in a subtle way.

Suppose, at an extreme, you have an instruction that's somewhere between 0 and 124 bytes.  So you have a sequence like this:

bb align 128:
ZERO_TO_124_BYTES_INSTR
B another_bb

bb2 align 128:
[...]

Say the start of the block is aligned.  We add up the size of the block: ZERO_TO_124_BYTES_INSTR is 124 bytes, B is 4 bytes, so the block is 128 bytes long.  Calling alignTo() does nothing, so we conclude there's no padding after the branch.  But actually, there can be up to 124 bytes of padding depending on the actual size of ZERO_TO_124_BYTES_INSTR.  And that padding increases the distance from the branch to the destination.

To get this right, we must not call alignTo(): trying to compute padding after a block using alignTo() assumes we know the exact offset of the end of the block, which is not a correct assumption.  We need to instead estimate the maximum amount of padding the .p2align directive will insert.

I'm not sure if there are any pseudo-instructions that are actually variable-length on aarch64 at the moment, but that's how the interface works in general; such instructions exist on other targets, and they've existed in the past on AArch64.  We also need to consider the interaction with MachineOutliner, which can effectively shrink the size of arbitrary sequences of instructions.

Branch relaxation is pretty forgiving, generally, so I'm not surprised nobody has tripped over this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150009