Ad -mrelax, -mno-relax flags to enable/disable RISCV linker relaxation
|1874 ↗||(On Diff #139757)|
I think we should define both -mrelax and -mno-relax flags
|55 ↗||(On Diff #139757)|
If we add both mrelax and mno-relax flags, we need to update this logic...
// -mrelax is default, unless -mno-relax is specified.
if(A->getOption().matches(options::OPT_mno_relax)) Relax = false;
Sorry, I misread D444886. Still, we should do Features.push_back("-relax") at least in the case where -mno-relax is given explicitly, meaning we don't have to rely on the backend default.
Thanks Kito. -mrelax and -mno-relax currently only affect the backend. For completeness, I think this patch needs to pass the appropriate flag to the linker depending on relax/no-relax.
Hi Alex. RISCVToolChain class does not define yet, so we can't pass --no-relax in RISCV::Linker::ConstructJob. Should we passing --no-relax when we introduce RISCVToolchain? Or there is another place we could implement passing RISCV linker flags?
I wonder if it would be safer to change this patch so it adds -mrelax and -mno-relax but doesn't compile with linker relaxation by default. That makes it easier to test linker relaxation support, and gives more time for testing before then flipping to -mrelax as the default.
|8 ↗||(On Diff #146737)|
We need a another RUN line and CHECK here to determine the default whether +relax is passed or not when the user specifies neither -mrelax or -mno-relax.
I saw that. I don't mean to be too nit-picky, but there's an advantage in also testing the default in this patch - it tests the current behaviour and makes it obvious in the follow-up patch that the behaviour actually changed.