search

riscv-non-isa / riscv-asm-manual

RISC-V Assembly Programmer's Manual
https://jira.riscv.org/browse/RVG-4
Creative Commons Attribution 4.0 International
1.44k stars 238 forks source link

Recommend avoiding .align #105

Closed Timmmm closed 4 months ago

Timmmm commented 4 months ago

Move the .p2align and .balign directives next to .align in the table, and add a note recommending avoiding .align in favour of the explicit directives which are much clearer.

Timmmm commented 4 months ago

Please remove the editorialization about .align. .align is standard stuff in GAS etc. I'm not unsympathetic to your point, but it's more important that RISC-V be compatible with other assembly languages than it is to scratch this itch.

It is still compatible with other assembly languages, at least in the sense that you can write .align. It's not compatible in the sense that it won't necessarily do the same thing though, which was the whole point of the comment. This is only a recommendation anyway, so I don't see how it could affect compatibility. Maybe I've missed your point.

cmuellner commented 4 months ago

Please remove the editorialization about .align. .align is standard stuff in GAS etc. I'm not unsympathetic to your point, but it's more important that RISC-V be compatible with other assembly languages than it is to scratch this itch.

The whole PR is just an editorialization about .align. Moving .p2align and .balign were already documented. So, dropping the editorialization means dropping the PR.

The GAS manual states:

The way the required alignment is specified varies from system to system.

So, there can't be compatibility with all other supported architectures/systems.

But I think I see what you mean. In fact, RISC-V defines .align in a reasonable way, and it did not cause the inconsistent ways in which .align was defined for other architectures in the past. Therefore, it is reasonable to question why we would recommend not using .align.

Timmmm commented 4 months ago

Therefore, it is reasonable to question why we would recommend not using .align.

Clearly my explanation wasn't as good as I thought! It's because the meaning of .align is not clear without carefully reading the manual, and this can lead to bugs and misunderstandings (not theoretical; I have seen this). I think many people would assume .align 4 means align to 4 bytes (which is not unreasonable given that it does mean that on other architectures!).

It technically doesn't make any difference to RISC-V how other architectures define .align, but the fact that they are inconsistent adds to the ambiguity.

I'm totally open to better ways to explain that if anyone can think of any. I was trying to keep it a bit briefer than this comment...

aswaterman commented 4 months ago

I meant editorialization in the sense that it reads like an opinion piece rather than a technical document :) I'd be fine with something like the following, along the lines of what @MaskRay was suggesting:

The .align directive for RISC-V is an alias to .p2align, which aligns to a power of two, so .align 2 means align to 4 bytes. Because the definition of the .align directive varies by architecture, it is recommended to use the unambiguous .p2align or .balign directives instead.

Timmmm commented 4 months ago

Updated, thanks for the suggestion!

MaskRay commented 4 months ago

Typo in the title: recommend

cmuellner commented 4 months ago

Merged. Thanks!