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.

Sign up for GitHub

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

Jump to bottom

Improper code generation for cross-compilation using arm-none-eabi-gcc #23817

Open
maleyva1 opened this issue Jul 10, 2024 · 5 comments
Open

Improper code generation for cross-compilation using arm-none-eabi-gcc #23817

maleyva1 opened this issue Jul 10, 2024 · 5 comments

Comments

@maleyva1
Copy link
Contributor

Description

Nim generates improper code for cross-compilation. The followingnim.cfg used:

cpu:arm
mm:arc
os:standalone
define:useMalloc
define:noSignalHandler
stackTrace:off
threads:off
lineTrace:off
assertions:off
checks:on
d:nimAllocPagesViaMalloc
d:nimPage512

arm.standalone.gcc.path = "/usr/bin/"
arm.standalone.gcc.exe = "arm-none-eabi-gcc"
arm.standalone.gcc.linkerexe = "arm-none-eabi-gcc"

Example that highlights this behavior:

proc main() =
  var t = 0
  inc(t)

when isMainModule:
  main()

Nim Version

Nim Compiler Version 2.0.8 [Linux: amd64]
Compiled at 2024-07-03
Copyright (c) 2006-2023 by Andreas Rumpf

git hash: 5935c3b
active boot switches: -d:release

Current Output

In file included from /home/mark/.cache/nim/test_d/@mtest.nim.c:7:
/home/mark/.cache/nim/test_d/@mtest.nim.c: In function 'main__test_u1':
/home/mark/.cache/nim/test_d/@mtest.nim.c:32:35: error: passing argument 3 of '__builtin_sadd_overflow' from incompatible pointer type [-Wincompatible-pointer-types]
   32 |         if (nimAddInt(t, ((NI)1), &TM__ipcYmBC9bj9a1BW35ABoB1Kw_2)) { raiseOverflow(); goto BeforeRet_;
      |                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                   |
      |                                   NI * {aka long int *}
/home/mark/.choosenim/toolchains/nim-2.0.8/lib/nimbase.h:576:64: note: in definition of macro 'nimAddInt'
  576 |     #define nimAddInt(a, b, res) __builtin_sadd_overflow(a, b, res)
      |                                                                ^~~
/home/mark/.cache/nim/test_d/@mtest.nim.c:32:35: note: expected 'int *' but argument is of type 'NI *' {aka 'long int *'}
   32 |         if (nimAddInt(t, ((NI)1), &TM__ipcYmBC9bj9a1BW35ABoB1Kw_2)) { raiseOverflow(); goto BeforeRet_;
      |                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/mark/.choosenim/toolchains/nim-2.0.8/lib/nimbase.h:576:64: note: in definition of macro 'nimAddInt'
  576 |     #define nimAddInt(a, b, res) __builtin_sadd_overflow(a, b, res)
      |                                                                ^~~
Error: execution of an external compiler program '/usr/bin/arm-none-eabi-gcc -c  -w -fmax-errors=3   -I/home/mark/.choosenim/toolchains/nim-2.0.8/lib -I/home/mark/Documents/Programming/nim/embedded -o /home/mark/.cache/nim/test_d/@mtest.nim.c.o /home/mark/.cache/nim/test_d/@mtest.nim.c' failed with exit code: 1

Expected Output

Valid generated code for cross-compiling with arm-none-eabi-gcc.

Possible Solution

No response

Additional Information

No response
@Araq
Copy link
Member

Araq commented Jul 10, 2024

NI should not be long int for your platform:

  result.addf("#define NIM_INTBITS $1\L", [
    platform.CPU[conf.target.targetCPU].intSize.rope])

nimbase.h:

typedef int32_t NI32;
...
#  elif NIM_INTBITS == 32
typedef NI32 NI;
...

@maleyva1
Copy link
Contributor Author

Unfortunately it appears that's how the arm-none-eabi-gcc compiler defines int32_t. Getting the GCC preprocessor output with -E reveals that int32_t is typedef'd to __int32_t and itself typedef'd to long int.

It is interesting to note that on GCC 13 (arm-none-eabi build), the above issue is a warning, but in 14 it is an error.

@Araq
Copy link
Member

Araq commented Jul 11, 2024

Well ok but then long int is 32 bits isn't it. I suppose the problem is that __builtin_sadd_overflow takes an int and GCC is super picky telling us that a long int is not an int even though both are 32 bits on your platform. See if things start to work if you patch nimbase.h so that NI is int and not long int.

@maleyva1
Copy link
Contributor Author

I suppose the problem is that __builtin_sadd_overflow takes an int and GCC is super picky telling us that a long int is not an int even though both are 32 bits on your platform

It appears this behavior is new in GCC 14 (refer to the "Type checking on pointer types" section) and only affects C standards after C89. With C89, it generates a warning (see here).

See if things start to work if you patch nimbase.h so that NI is int and not long int.

That works, but I'm unsure if patching NI to int on arm-none-eabi platforms is a good long-term solution given the information above. I don't mind submitting the patch I have locally, but let me know what you think.

@Araq
Copy link
Member

Araq commented Jul 14, 2024

Hmm why don't we simply use __builtin_saddl_overflow and the like...

maleyva1 added a commit to maleyva1/Nim that referenced this issue Jul 14, 2024
@maleyva1
fix nim-lang#23817; Use __builtin_saddl_overflow variants for arm-non…
7ed1ca9 
…e-eabi-gcc.
@maleyva1 maleyva1 mentioned this issue Jul 14, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@Araq @maleyva1