feat: Support R_RISCV_ALIGN relaxation - #1772
Conversation
| ensure!( | ||
| addend.is_power_of_two(), | ||
| "A power of 2 expected for Alignment relocation: {}", | ||
| addend | ||
| ); |
There was a problem hiding this comment.
This assertion was incorrect. The addend here represents the number of NOP bytes to be inserted, not an alignment value, so there's no guarantee it's a power of two.
| ) -> Option<Self::Relaxation>; | ||
|
|
||
| /// Fill `len` bytes of NOP padding at `offset` in `buf`. | ||
| fn fill_nop_padding(_buf: &mut [u8], _offset: usize, _len: usize) {} |
There was a problem hiding this comment.
While RISC-V is currently the only architecture using RelocationKind::Alignment, it's defined generically to allow for future use by other architectures.
| fn fill_nop_padding(buf: &mut [u8], offset: usize, len: usize) { | ||
| let mut i = 0; | ||
| while i + 4 <= len { | ||
| buf[offset + i..offset + i + 4].copy_from_slice(&0x0000_0013u32.to_le_bytes()); | ||
| i += 4; | ||
| } | ||
| if i + 2 <= len { | ||
| buf[offset + i..offset + i + 2].copy_from_slice(&0x0001u16.to_le_bytes()); | ||
| } | ||
| } |
There was a problem hiding this comment.
This fills the padding area with as many 4-byte NOP instructions as possible, then fills the remaining bytes with c.nop. Since only scenarios where two bytes remain would occur when C extensions are enabled, this method should remain valid even when supporting architectures other than rv64gc in the future.
There was a problem hiding this comment.
That seems like a useful comment. Do you think it'd be worthwhile adding something along those lines as a comment in the code?
| fn fill_nop_padding(buf: &mut [u8], offset: usize, len: usize) { | ||
| let mut i = 0; | ||
| while i + 4 <= len { | ||
| buf[offset + i..offset + i + 4].copy_from_slice(&0x0000_0013u32.to_le_bytes()); | ||
| i += 4; | ||
| } | ||
| if i + 2 <= len { | ||
| buf[offset + i..offset + i + 2].copy_from_slice(&0x0001u16.to_le_bytes()); | ||
| } | ||
| } |
There was a problem hiding this comment.
That seems like a useful comment. Do you think it'd be worthwhile adding something along those lines as a comment in the code?
|
|
||
| // The alignment the assembler requested. | ||
| let alignment = addend.next_power_of_two(); | ||
| let desired = (p + alignment - 1) & !(alignment - 1); // align_up |
There was a problem hiding this comment.
Might be more readable to use next_multiple_of
The assembler emits
R_RISCV_ALIGNrelocations with a NOP sled whose byte length is stored in the addend. The linker must delete excess NOPs so that the instruction after the sled lands on the required alignment boundary.part of #874