Document the non-atomic relaxation pitfall about riscv-elf-psabi-doc HOT 16 OPEN

Yes, I can see how this could be a potential pitfall, but is there a better example? I believe ld currently doesn't relax %hi/%lo pairs into gprel for symbols in .text.

Would it make sense for %lo to link with its corresponding %hi à la %pcrel_lo? That should make it obvious that the pair needs to be relaxed together.

from riscv-elf-psabi-doc.

I think that if you don't perform gprel relaxations for anything outside the data segment and don't have relaxations of any kind inside the data segment, that's probably good enough (R_RISCV_CALL is already atomic).

Threading the %lo came up as a possibility during the binutils upstreaming; we decided it was too much complication for very little benefit.

from riscv-elf-psabi-doc.

The plan is to move to a two-pass, R_RISCV_DELETE-based algorithm for all the relaxations, which is why those patches are still WIP (I want to make sure it can be used for something else before submitting). We're just time limited right now.

from riscv-elf-psabi-doc.

Can a gcc contributor advice whether -mno-relax can affect code generation?
This is a controversial point in the Clang patch (if -mno-relax, add .option norelax?): https://reviews.llvm.org/D102535

Does the code generator reserve rights to make -mno-relax change code generation behavior?

from riscv-elf-psabi-doc.

@jim-wilson

from riscv-elf-psabi-doc.

-mno-relax doesn't affect code generation. As you mentioned, gcc -S -mno-relax tmp.c; gcc -c tmp.s should work which is why we need the .option norelax.

from riscv-elf-psabi-doc.

gcc -S -mabi=foo tmp.c; gcc -c tmp.s doesn't work, why should it for -mno-relax? If you use different flags to assemble than what you generated the code with that's on you and you should fix your broken build system.

from riscv-elf-psabi-doc.

Also, who on earth is invoking the assembler separately? Sure, we can include .option norelax, but it just masks the compiler driver not forwarding -mno-relax on to the assembler (which used to be the case for GCC, I don't know if it finally got fixed: this actually meant gcc -c -mno-relax tmp.s didn't do anything for a very long time and you had to pass -Wa,-mno-relax anyway to assemble files without relaxation support).

from riscv-elf-psabi-doc.

This is a pointless argument. I'm not changing how the GNU toolchain works.

from riscv-elf-psabi-doc.

It's not pointless. https://github.com/riscv/riscv-c-api-doc documents various compiler flags, and we need to have a common interface that can be relied upon from GCC and Clang (and Mark will tell you that too). Both are free to offer stronger semantics and extra options than the other, but we need to have compatibility between the two, otherwise we are doing users a disservice. You maintain GCC, we maintain LLVM, and so together we somehow need to reach an agreement over what is the best approach. The "this is what GCC does, that's what you must do" is not that; the two compilers should be on an equal footing, and arguably LLVM is even more important these days due to its ability to be reused as the backend for other projects, though GCC remains the dominant compiler in the Linux world for now.

from riscv-elf-psabi-doc.

GCC works the way it does because it evolved over 10 years or so. I can't go back in time and change history. You just need to accept that some things don't work the way that they do if we had designed it from scratch knowing what we know now. The .option norelax is one of those. There is really no reason to waste a large amount of time arguing about this. It is a completely trivial issue. It is just one line emitted by the compiler into the assembly language file. This isn't worth an argument, or a possibly backward compatibility breaking change. Just accept it for what it is and move forward.

from riscv-elf-psabi-doc.

I'm not asking for GCC to change, I'm just saying Clang may well not follow GCC in permitting such lax programming practices, provided there's not a good argument against it besides "GCC lets you do $stupid_thing".

from riscv-elf-psabi-doc.

(and this query was not about "GCC should change", it was "is our understanding of the flag, and how it could possibly be used by GCC, correct?")

from riscv-elf-psabi-doc.

I don't see how anything can be misunderstood here. You already have a clang patch that correctly implements this, you are just refusing to accept it.

If the user specifies -mno-relax, then we put .option norelax in the assembly file. Unlike an ABI, relaxation can be turned off and on at will in the middle of an assemblly file, and this is actually a necessary feature to make relaxation work properly, as the startfile initializing gp needs to temporarily turn relaxation off. So if you have both a command line option and a .option relax/norelax in the assembly file, then the one in the assembly file takes precedence. This is all trivial to understand.

A feature should be judged on its merits, not theoretical discussions about the "proper" way to write code. For this feature, it is two trivial lines of code in the compiler, so the cost of keeping the feature is effectively zero. If we drop it, then there is the possibilty that some user code will break as this is a non-backward compatible change. Even worse than that, code will be silently compiled wrong which is the absolute worst possible mistake that a compiler can make. Therefore there is nothing to debate here. The feature must be kept.

Talking about lax programming practices is inappropriate here. We support many different kinds of programming, embedded and linux. We support may different kinds of build systems. Assuming that there is only one correct way to write code is wrong. Also, it is wrong to assume that people have perfect code, perfect build system, and infinite amounts of time to fix issues with their code. In the real world, there are always things that are non optimal, and the toolchain should be willing to make things easier for programmers, rather than add unnecessary obstacles in their way.

I see no reason why we should even be debating this feature here. Assembly language formats are always ugly, and often have minor incompatiibilities across compilers. clang goes directly from C to object files, so the assembly language format shouldn't even matter to clang except in uncommon cases.

There are real substantive issues that the psABI committee should be working on. But instead we are wasting time arguing about this trivial and unimportant feature that can't be changed now. We all have limited time to spend dealing with psABI issues. We should be using that time to discuss important issues. We should not be wasting time with trivial unimportant issues that aren't even broken.

It seems the only reason we are discussing this is because you are unable to compromise in any situation no matter how trivial or unimportant. The ability to compromise is a very important engineering skill. This is a skill you need to learn. And you can start by accepting the clang patch you already have.

from riscv-elf-psabi-doc.

I don't see how anything can be misunderstood here. You already have a clang patch that correctly implements this, you are just refusing to accept it.

If the user specifies -mno-relax, then we put .option norelax in the assembly file. Unlike an ABI, relaxation can be turned off and on at will in the middle of an assemblly file, and this is actually a necessary feature to make relaxation work properly, as the startfile initializing gp needs to temporarily turn relaxation off. So if you have both a command line option and a .option relax/norelax in the assembly file, then the one in the assembly file takes precedence. This is all trivial to understand.

A feature should be judged on its merits, not theoretical discussions about the "proper" way to write code. For this feature, it is two trivial lines of code in the compiler, so the cost of keeping the feature is effectively zero. If we drop it, then there is the possibilty that some user code will break as this is a non-backward compatible change. Even worse than that, code will be silently compiled wrong which is the absolute worst possible mistake that a compiler can make. Therefore there is nothing to debate here. The feature must be kept.

Talking about lax programming practices is inappropriate here. We support many different kinds of programming, embedded and linux. We support may different kinds of build systems. Assuming that there is only one correct way to write code is wrong. Also, it is wrong to assume that people have perfect code, perfect build system, and infinite amounts of time to fix issues with their code. In the real world, there are always things that are non optimal, and the toolchain should be willing to make things easier for programmers, rather than add unnecessary obstacles in their way.

I see no reason why we should even be debating this feature here. Assembly language formats are always ugly, and often have minor incompatiibilities across compilers. clang goes directly from C to object files, so the assembly language format shouldn't even matter to clang except in uncommon cases.

There are real substantive issues that the psABI committee should be working on. But instead we are wasting time arguing about this trivial and unimportant feature that can't be changed now. We all have limited time to spend dealing with psABI issues. We should be using that time to discuss important issues. We should not be wasting time with trivial unimportant issues that aren't even broken.

It seems the only reason we are discussing this is because you are unable to compromise in any situation no matter how trivial or unimportant. The ability to compromise is a very important engineering skill. This is a skill you need to learn. And you can start by accepting the clang patch you already have.

Personal attacks and telling Clang/LLVM maintainers what patches to accept are both entirely inappropriate. Please refrain from making such comments ever again.

from riscv-elf-psabi-doc.

Personal attacks and telling Clang/LLVM maintainers what patches to accept are both entirely inappropriate. Please refrain from making such comments ever again.

Obviously, your "GCC lets you do $stupid_thing" remark falls into one or both of those buckets. You need to take it down a notch, too.

from riscv-elf-psabi-doc.