lop-microblaze-riscv: Restore f/d FPU suffix in bsp_archflags - #829
Conversation
Commit 22551d4 removed bsp_archflags.append('f'/'d') while keeping the equivalent appends for archflags, causing -mabi=lp64 in linkflags instead of -mabi=lp64d for MicroBlaze-V cores with FPU enabled. This results in a link-time failure: can't link double-float modules with soft-float modules Restore the missing bsp_archflags.append() calls so that linker flags stay in sync with compiler flags when FPU is present. Signed-off-by: M Lakshmaiah <lakshmaiah.m@amd.com>
zeddii
left a comment
There was a problem hiding this comment.
Verified, and the fix is right. Worth recording why on the PR, because the commit this reverses reads convincingly and someone could reasonably re-apply it later.
22551d46 ("Omit f and d from -march compiler and linker flags") justified itself with "these are not a valid in -march option". That's true of -march, but it isn't what these appends were doing. The f/d land on the list after the -mabi=lp64 token, so ''.join() extends the ABI string rather than the ISA string:
archflags = ['-march=', 'rv64i_zicsr_zifencei', ' -mabi=lp64', 'd']
-> "-march=rv64i_zicsr_zifencei -mabi=lp64d"
They were never -march components. What the commit actually did was remove them from one of the two lists, so the compiler kept getting lp64d while the linker dropped to lp64:
master compiler: -march=rv64i_zicsr_zifencei -mabi=lp64d
linker : -march=rv64i_zicsr_zifencei -mabi=lp64 <- mismatch
with #829 compiler: -march=rv64i_zicsr_zifencei -mabi=lp64d
linker : -march=rv64i_zicsr_zifencei -mabi=lp64d <- match
Which is exactly the "can't link double-float modules with soft-float modules" you're reporting, since bsp_linkflags feeds cflags_data['linkflags'] on line 194.
One structural note inline — not for this PR, but this class of bug is going to come back.
| bsp_archflags.append('f') | ||
| elif n['xlnx,use-fpu'].value[0] == 2: | ||
| archflags.append('d') | ||
| bsp_archflags.append('d') |
There was a problem hiding this comment.
Follow-up thought rather than anything to change here: your two lines are the correct fix.
With these restored, every single append in this block writes to both lists: the tune, b, _zicsr_zifencei, _zba, _zbb, _zbs, _zbc, _zicbom, all three -mabi= variants, and now f/d. There is no point where archflags and bsp_archflags are meant to differ — they're identical by construction, kept in step by hand across roughly a dozen paired calls.
That pairing is what allowed the original regression: 22551d46 removed one half of one pair, and nothing could detect it, because the only thing asserting the two lists agree is that someone remembered to type both lines.
Building one list and deriving the other at the join — or just using one, given they're the same string — would make this class of bug unrepresentable rather than merely fixed. Worth a separate patch if you're already in this file.
Commit 22551d4 removed bsp_archflags.append('f'/'d') while keeping the equivalent appends for archflags, causing -mabi=lp64 in linkflags instead of -mabi=lp64d for MicroBlaze-V cores with FPU enabled. This results in a link-time failure:
can't link double-float modules with soft-float modules
Restore the missing bsp_archflags.append() calls so that linker flags stay in sync with compiler flags when FPU is present.