[BOLT] Return proper minimal alignment from BF #67707
Conversation
|
@maksfb I fill like setting minimal required alignment per BF is more proper way to fix the CI alignment issue to be honest. So we would have "minimal alignment" field that is the BFs guaranteed alignment that we would have and the alignment/maxbyte alignment non guaranteed logic. What do you think? Thanks! |
Sounds like we should handle CIs separately from BFs. |
|
A few years ago I had the same feelings. Right now I'm not really sure how to re-design the code to handle them separately to be honest. |
|
Do you anticipate different CIs to have different min alignments in the future? If not, I would rather modify the newly added |
|
Well, what's came in to my mind is the golang functions, that needs at least the 16 bytes alignment. But I agree that for now we can check everything and return the result from getMinAlignment() function, let me update the review later. Thanks! |
Currently minimal alignment of function is hardcoded to 2 bytes. Add 2 more cases: 1. In case BF is data in code return the alignment of CI as minimal alignment 2. For aarch64 and riscv platforms return the minimal value of 4 (added test for aarch64) Otherwise fallback to returning the 2 as it previously was.
yota9
changed the title
|
Gentle ping |
|
@maksfb gentle ping |
In llvm#67707, the minimum function alignment on RISC-V was set to 4. When RVC (compressed instructions) is enabled, the minimum alignment can be reduced to 2. This patch implements this by delegating the choice of minimum alignment to a new `MCPlusBuilder::getMinFunctionAlignment` function. This way, the target-dependent code in `BinaryFunction` is minimized.
In #67707, the minimum function alignment on RISC-V was set to 4. When RVC (compressed instructions) is enabled, the minimum alignment can be reduced to 2. This patch implements this by delegating the choice of minimum alignment to a new `MCPlusBuilder::getMinFunctionAlignment` function. This way, the target-dependent code in `BinaryFunction` is minimized.
Currently minimal alignment of function is hardcoded to 2 bytes.
Add 2 more cases:
alignment
test for aarch64)
Otherwise fallback to returning the 2 as it previously was.