Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RISC-V64: command line argument --riscvfloat does not work #1567

Open
Release-Candidate opened this issue Oct 19, 2024 · 11 comments
Open

RISC-V64: command line argument --riscvfloat does not work #1567

Release-Candidate opened this issue Oct 19, 2024 · 11 comments
Labels
Additional info please Further information is requested Bug Something isn't working
Milestone

Comments

@Release-Candidate
Copy link

The files generated by ./c3c --target linux-riscv64 --riscvfloat=double compile use the soft-float ABI instead of the double-float ABI on RISC-V64. This leads to the following errors:

./c3c --target linux-riscv64 --riscvfloat=double compile ../resources/examples/hash.c3
...
ld.lld: error: std.hash.o: cannot link object files with different floating-point ABI from /usr/lib/riscv64-linux-gnu/Scrt1.o
ld.lld: error: std.math.o: cannot link object files with different floating-point ABI from /usr/lib/riscv64-linux-gnu/Scrt1.o
ld.lld: error: std.ascii.o: cannot link object files with different floating-point ABI from /usr/lib/riscv64-linux-gnu/Scrt1.o
...

An object file generated by the above compiler invocation:

$ file std.core.mem.o
std.core.mem.o: ELF 64-bit LSB relocatable, UCB RISC-V, soft-float ABI, version 1 (SYSV), with debug_info, not stripped

but should be:

$ file /usr/lib/riscv64-linux-gnu/Scrt1.o
/usr/lib/riscv64-linux-gnu/Scrt1.o: ELF 64-bit LSB relocatable, UCB RISC-V, RVC, double-float ABI, version 1 (SYSV), for GNU/Linux 4.15.0, not stripped
@lerno lerno added the Bug Something isn't working label Oct 20, 2024
@lerno
Copy link
Collaborator

lerno commented Oct 20, 2024

From what I read, setting it to double should indeed set compiler.platform.riscv.flen = 8, perhaps there is some additional flag that has to be packed into the LLVM IR for this. Could you perhaps dump the LLVM IR for a proper C compilation of a particular function that fails with C3?

@lerno lerno added the Additional info please Further information is requested label Oct 20, 2024
@Release-Candidate
Copy link
Author

Release-Candidate commented Oct 20, 2024

This is a flag in the ELF header: https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-elf.adoc#e-flags-layout

LLVM IR:
It's in the "target-features", I guess. LLVM sets +64bit,+a,+c,+d,+f,+m,+relax,+zicsr,+zmmul if i understand that correctly, "d" I guess means RISC-V double support.
Edit: no, thats !1 = !{i32 1, !"target-abi", !"lp64d"}
But could be both, as C3 sets neither.

int main(void) { return 0; }
; ModuleID = 'main.c'
source_filename = "main.c"
target datalayout = "e-m:e-p:64:64-i64:64-i128:128-n32:64-S128"
target triple = "riscv64-unknown-linux-gnu"

; Function Attrs: noinline nounwind optnone uwtable
define dso_local signext i32 @main() #0 {
  %1 = alloca i32, align 4
  store i32 0, ptr %1, align 4
  ret i32 0
}

attributes #0 = { noinline nounwind optnone uwtable "frame-pointer"="all" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="generic-rv64" "target-features"="+64bit,+a,+c,+d,+f,+m,+relax,+zicsr,+zmmul,-b,-e,-experimental-smmpm,-experimental-smnpm,-experimental-ssnpm,-experimental-sspm,-experimental-ssqosid,-experimental-supm,-experimental-zacas,-experimental-zalasr,-experimental-zicfilp,-experimental-zicfiss,-h,-shcounterenw,-shgatpa,-shtvala,-shvsatpa,-shvstvala,-shvstvecd,-smaia,-smcdeleg,-smcsrind,-smepmp,-smstateen,-ssaia,-ssccfg,-ssccptr,-sscofpmf,-sscounterenw,-sscsrind,-ssstateen,-ssstrict,-sstc,-sstvala,-sstvecd,-ssu64xl,-svade,-svadu,-svbare,-svinval,-svnapot,-svpbmt,-v,-xcvalu,-xcvbi,-xcvbitmanip,-xcvelw,-xcvmac,-xcvmem,-xcvsimd,-xsfcease,-xsfvcp,-xsfvfnrclipxfqf,-xsfvfwmaccqqq,-xsfvqmaccdod,-xsfvqmaccqoq,-xsifivecdiscarddlone,-xsifivecflushdlone,-xtheadba,-xtheadbb,-xtheadbs,-xtheadcmo,-xtheadcondmov,-xtheadfmemidx,-xtheadmac,-xtheadmemidx,-xtheadmempair,-xtheadsync,-xtheadvdot,-xventanacondops,-xwchc,-za128rs,-za64rs,-zaamo,-zabha,-zalrsc,-zama16b,-zawrs,-zba,-zbb,-zbc,-zbkb,-zbkc,-zbkx,-zbs,-zca,-zcb,-zcd,-zce,-zcf,-zcmop,-zcmp,-zcmt,-zdinx,-zfa,-zfbfmin,-zfh,-zfhmin,-zfinx,-zhinx,-zhinxmin,-zic64b,-zicbom,-zicbop,-zicboz,-ziccamoa,-ziccif,-zicclsm,-ziccrse,-zicntr,-zicond,-zifencei,-zihintntl,-zihintpause,-zihpm,-zimop,-zk,-zkn,-zknd,-zkne,-zknh,-zkr,-zks,-zksed,-zksh,-zkt,-ztso,-zvbb,-zvbc,-zve32f,-zve32x,-zve64d,-zve64f,-zve64x,-zvfbfmin,-zvfbfwma,-zvfh,-zvfhmin,-zvkb,-zvkg,-zvkn,-zvknc,-zvkned,-zvkng,-zvknha,-zvknhb,-zvks,-zvksc,-zvksed,-zvksg,-zvksh,-zvkt,-zvl1024b,-zvl128b,-zvl16384b,-zvl2048b,-zvl256b,-zvl32768b,-zvl32b,-zvl4096b,-zvl512b,-zvl64b,-zvl65536b,-zvl8192b" }

!llvm.module.flags = !{!0, !1, !2, !4, !5, !6, !7, !8}
!llvm.ident = !{!9}

!0 = !{i32 1, !"wchar_size", i32 4}
!1 = !{i32 1, !"target-abi", !"lp64d"}
!2 = !{i32 6, !"riscv-isa", !3}
!3 = !{!"rv64i2p1_m2p0_a2p1_f2p2_d2p2_c2p0_zicsr2p0_zmmul1p0"}
!4 = !{i32 8, !"PIC Level", i32 2}
!5 = !{i32 7, !"PIE Level", i32 2}
!6 = !{i32 7, !"uwtable", i32 2}
!7 = !{i32 7, !"frame-pointer", i32 2}
!8 = !{i32 8, !"SmallDataLimit", i32 8}
!9 = !{!"Debian clang version 19.1.2 (1)"}

C3:

module hello_world;

fn void main()
{
}
; ModuleID = 'hello_world'
source_filename = "hello_world"
target datalayout = "e-m:e-p:64:64-i64:64-i128:128-n32:64-S128"
target triple = "riscv64-unknown-linux"

; Function Attrs: nounwind ssp uwtable
define void @hello_world.main() #0 !dbg !8 {
entry:
  ret void
}

; Function Attrs: nounwind ssp uwtable
define signext i32 @main(i32 signext %0, ptr %1) #0 !dbg !11 {
entry:
  %.anon = alloca i32, align 4
  %.anon1 = alloca ptr, align 8
  %.anon2 = alloca i32, align 4
  %.anon3 = alloca ptr, align 8
  store i32 %0, ptr %.anon, align 4
    #dbg_declare(ptr %.anon, !19, !DIExpression(), !20)
  store ptr %1, ptr %.anon1, align 8
    #dbg_declare(ptr %.anon1, !21, !DIExpression(), !20)
  %2 = load i32, ptr %.anon, align 4
  store i32 %2, ptr %.anon2, align 4
  %3 = load ptr, ptr %.anon1, align 8
  store ptr %3, ptr %.anon3, align 8
  call void @hello_world.main(), !dbg !22
  ret i32 0, !dbg !25
}

attributes #0 = { nounwind ssp uwtable "frame-pointer"="all" "no-trapping-math"="true" "stack-protector-buffer-size"="8" }

!llvm.module.flags = !{!0, !1, !2, !3, !4, !5}
!llvm.dbg.cu = !{!6}

!0 = !{i32 4, !"PIE Level", i32 2}
!1 = !{i32 4, !"PIC Level", i32 2}
!2 = !{i32 2, !"Dwarf Version", i32 4}
!3 = !{i32 2, !"Debug Info Version", i32 3}
!4 = !{i32 2, !"frame-pointer", i32 2}
!5 = !{i32 1, !"uwtable", i32 2}
!6 = distinct !DICompileUnit(language: DW_LANG_C11, file: !7, producer: "c3c", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false)
!7 = !DIFile(filename: "main.c3", directory: "/home/roland/code/c3c/build")
!8 = distinct !DISubprogram(name: "main", linkageName: "hello_world.main", scope: !7, file: !7, line: 3, type: !9, scopeLine: 3, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !6)
!9 = !DISubroutineType(types: !10)
!10 = !{null}
!11 = distinct !DISubprogram(name: "_$main", linkageName: "main", scope: !7, file: !7, line: 3, type: !12, scopeLine: 3, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !6, retainedNodes: !18)
!12 = !DISubroutineType(types: !13)
!13 = !{!14, !14, !15}
!14 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
!15 = !DIDerivedType(tag: DW_TAG_pointer_type, name: "char**", baseType: !16, size: 64, align: 64, dwarfAddressSpace: 0)
!16 = !DIDerivedType(tag: DW_TAG_pointer_type, name: "char*", baseType: !17, size: 64, align: 64, dwarfAddressSpace: 0)
!17 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_unsigned_char)
!18 = !{}
!19 = !DILocalVariable(name: ".anon", arg: 1, scope: !11, file: !7, line: 3, type: !14)
!20 = !DILocation(line: 3, column: 9, scope: !11)
!21 = !DILocalVariable(name: ".anon", arg: 2, scope: !11, file: !7, line: 3, type: !15)
!22 = !DILocation(line: 18, column: 2, scope: !23, inlinedAt: !20)
!23 = distinct !DISubprogram(name: "@main_to_void_main", linkageName: "@main_to_void_main", scope: !24, file: !24, line: 16, scopeLine: 16, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition, unit: !6)
!24 = !DIFile(filename: "main_stub.c3", directory: "/home/roland/code/c3c/lib/std/core/private")
!25 = !DILocation(line: 19, column: 9, scope: !23, inlinedAt: !20)

@Release-Candidate
Copy link
Author

Well, I guess there should be the ABI setup : https://github.com/c3lang/c3c/blob/master/src/compiler/target.c#L2016

@Release-Candidate
Copy link
Author

Could you perhaps dump the LLVM IR for a proper C compilation of a particular function that fails with C3?

And just to make that clear: the compiler does not generate working binaries at_all on a RISC-V CPU which supports either 32 or 64 bit floats.

@lerno
Copy link
Collaborator

lerno commented Oct 20, 2024

From what I can see, these are the possible ABI types:

  .Case("ilp32", ABI_ILP32)
                       .Case("ilp32f", ABI_ILP32F)
                       .Case("ilp32d", ABI_ILP32D)
                       .Case("ilp32e", ABI_ILP32E)
                       .Case("lp64", ABI_LP64)
                       .Case("lp64f", ABI_LP64F)
                       .Case("lp64d", ABI_LP64D)
                       .Case("lp64e", ABI_LP64E)

The f/d variants are clear, but what is e, and what is the difference between i and non-i?

@Release-Candidate
Copy link
Author

Release-Candidate commented Oct 20, 2024

If I've read that correctly,E is the RVE (Embedded) variant, which has only half the registers, so 16 instead of 32.

i stands for int, ilp32 means int, long and pointers are 32 bits, lp64: long and pointers are 64 bit.

Oh, I've found that regarding the E: https://reviews.llvm.org/D70401?id=233103

Btw. you really want to make lp64 and linux-riscv64 the default when running on Linux 64 bit (and the same for 32).

Unable to detect the default target, please set an explicit --target value. - there is no need for that on other archs on Linux.

@lerno
Copy link
Collaborator

lerno commented Oct 20, 2024

Ok, so hang on, let's see if we can break this down:

  1. floats: none on riscv32 => ilp32 riscv64 => lp64
  2. float => ilp32f / lp64f
  3. double => ilp32d / lp64d
  4. embedded => should this be another riscvfloat setting or something else?

Should riscvfloat be changed to abi? And we have default/float/double/embedded?

@lerno
Copy link
Collaborator

lerno commented Oct 20, 2024

(We don't have a CI for Riscv, so everything is a bit experimental BTW, it would be great to have something, all we have is a baremetal riscv test)

@Release-Candidate
Copy link
Author

Release-Candidate commented Oct 21, 2024

Ok, so hang on, let's see if we can break this down:

  1. floats: none on riscv32 => ilp32 riscv64 => lp64
  2. float => ilp32f / lp64f
  3. double => ilp32d / lp64d
  4. embedded => should this be another riscvfloat setting or something else?

Should riscvfloat be changed to abi? And we have default/float/double/embedded?

To be honest, I do not know why GCC and LLVM do it that way (with the ABI string). As you have to somehow set the ISA extensions anyway, other ABIs either are not possible at all or make no sense.
For C3 I would ignore the float ABI and use a sensible default one (double for RV64GC, "soft" for RV32I and anything RVE) and add the possibility to set what GCC and LLVM call "architecture", that is, the supported extensions. While there is the possibility of a freestanding RISC-V program or a RISC-V OS that uses such an unusual float ABI, I'd say there are more important things to support with C3 at first, like ISA extensions (the LLVM backend supports) - I have not seen a possibility to set these on the command line. I mean, theoretically there are 128 bit RISC-V implementations too, and if Windows runs on RISC-V there will be llp64 (32 bit longs and 64 bit long longs), but ...
For 64 bit you want at least RV64GC and optionally support the vector extension, so RV64GCV, as you have "native" vectors in the language. And these should use the double float ABI.

The list of extensions: https://lf-riscv.atlassian.net/wiki/spaces/HOME/pages/16154732/Ratified+Extensions

The (current) official documents: https://github.com/riscv/riscv-isa-manual/releases/tag/riscv-isa-release-ae98787-2024-10-19

And additionally the overview of all standard extensions (single letter extensions): https://en.wikichip.org/wiki/risc-v/standard_extensions - although there is e.g. RV64E missing on that site

@lerno
Copy link
Collaborator

lerno commented Oct 23, 2024

So what would you propose?

@Release-Candidate
Copy link
Author

So what would you propose?

To keep the command line minimal (like for x86_64) and not add every single RISC-V ISA extensions, I'd say:

Ditch the float ABI command line argument and use the float ABI suitable for the selected ISA:

  • double for RV64GC, RV64GCV and RVA23U64

I don't know too much about 32 bit RISC-V, I'd start with

  • soft for RVI32

On RISC-V64, make RV64GC the default for now1 and maybe in the near future (some years) switch to RVA23U64 as the default, when it is widely supported by CPUs. Add command line arguments to set RV64GCV and RVA23U64 (this is RV64GCV with additional extensions, see2). As for RISC-V32, I'd guess make RVI32 the default.

Footnotes

  1. two days ago, the RV23 profile has been ratified: https://riscv.org/announcements/2024/10/risc-v-announces-ratification-of-the-rva23-profile-standard/

  2. the page with the details of RVA23U64: https://github.com/riscv/riscv-profiles/blob/main/src/rva23-profile.adoc

@lerno lerno added this to the 0.6.5 milestone Nov 4, 2024
@lerno lerno modified the milestones: 0.6.5, 0.6.6 Dec 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Additional info please Further information is requested Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants