You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/01/25 08:15:09 UTC
[GitHub] [incubator-nuttx] xiaoxiang781216 opened a new pull request #5332: arch/Toolchain.defs: Simplify addition builtin library to EXTRA_LIBS
xiaoxiang781216 opened a new pull request #5332:
URL: https://github.com/apache/incubator-nuttx/pull/5332
## Summary
## Impact
No, refactor
## Testing
Pass CI
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #5332: arch/Toolchain.defs: Simplify addition builtin library to EXTRA_LIBS
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #5332:
URL: https://github.com/apache/incubator-nuttx/pull/5332#issuecomment-1024881579
> @xiaoxiang781216 I noticed that sama5d4-ek:ksh build failed. Could you confirm what is happening now?
>
Because ARCHCPUFLAGS overwrite by board's Make.defs:
https://github.com/apache/incubator-nuttx/blob/master/boards/arm/sama5/sama5d4-ek/scripts/Make.defs#L51
so the flags used to find the library in Toolchain.defs is different from the compiling.
@masayuki2009 Could you try https://github.com/apache/incubator-nuttx/pull/5371?
Do you know why CI doesn't catch this error before merge?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] AlanRosenthal commented on a change in pull request #5332: arch/Toolchain.defs: Simplify addition builtin library to EXTRA_LIBS
Posted by GitBox <gi...@apache.org>.
AlanRosenthal commented on a change in pull request #5332:
URL: https://github.com/apache/incubator-nuttx/pull/5332#discussion_r792173427
##########
File path: arch/arm/src/arm/Toolchain.defs
##########
@@ -103,21 +103,12 @@ OBJDUMP = $(CROSSDEV)objdump
# Add the builtin library
-EXTRA_LIBS += -lgcc
-EXTRA_LIBPATHS += -L "${shell dirname "`$(CC) $(ARCHCPUFLAGS) --print-libgcc-file-name`"}"
+EXTRA_LIBS := ${shell $(CC) $(ARCHCPUFLAGS) -print-file-name=libgcc.a}
ifneq ($(CONFIG_LIBM),y)
- LIBM_PATH = "${shell dirname "`$(CC) $(ARCHCPUFLAGS) --print-file-name=libm.a`"}"
-
- # Check if libm is provided by the compiler
-
- ifneq ($(LIBM_PATH),".")
- EXTRA_LIBS += -lm
- EXTRA_LIBPATHS += -L $(LIBM_PATH)
- endif
+ EXTRA_LIBS += ${wildcard ${shell $(CC) $(ARCHCPUFLAGS) --print-file-name=libm.a}}
Review comment:
why wildcard on this line, but not the others?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] masayuki2009 commented on pull request #5332: arch/Toolchain.defs: Simplify addition builtin library to EXTRA_LIBS
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #5332:
URL: https://github.com/apache/incubator-nuttx/pull/5332#issuecomment-1024874789
@xiaoxiang781216
I noticed that sama5d4-ek:ksh build failed.
Could you confirm what is happening now?
```
LD: nuttx
arm-none-eabi-ld --entry=__start -T/mnt/m2ssd/opensource/github_masayuki2009/apache-nuttx-sama5-knsh/nuttx/boards/arm/sama5/sama5d4-ek/scripts/dramboot.ld -L"/mnt/m2ssd/opensource/github_masayuki2009/apache-nuttx-sama5-knsh/nuttx/staging" -L"/mnt/m2ssd/opensource/github_masayuki2009/apache-nuttx-sama5-knsh/nuttx/arch/arm/src/board" -o "/mnt/m2ssd/opensource/github_masayuki2009/apache-nuttx-sama5-knsh/nuttx/nuttx" arm_vectortab.o --start-group -lsched -ldrivers -lboards -lstubs -lkc -lkmm -lkarch -lfs -lbinfmt -lboard /opt/gcc-arm-none-eabi-9-2020-q2-update/bin/../lib/gcc/arm-none-eabi/9.3.1/thumb/v7-a+simd/hard/libgcc.a /opt/gcc-arm-none-eabi-9-2020-q2-update/bin/../lib/gcc/arm-none-eabi/9.3.1/../../../../arm-none-eabi/lib/thumb/v7-a+simd/hard/libm.a --end-group
arm-none-eabi-ld: error: /opt/gcc-arm-none-eabi-9-2020-q2-update/bin/../lib/gcc/arm-none-eabi/9.3.1/thumb/v7-a+simd/hard/libgcc.a(_udivmoddi4.o) uses VFP register arguments, /mnt/m2ssd/opensource/github_masayuki2009/apache-nuttx-sama5-knsh/nuttx/nuttx does not
arm-none-eabi-ld: failed to merge target specific data of file /opt/gcc-arm-none-eabi-9-2020-q2-update/bin/../lib/gcc/arm-none-eabi/9.3.1/thumb/v7-a+simd/hard/libgcc.a(_udivmoddi4.o)
Makefile:164: recipe for target 'nuttx' failed
make[1]: *** [nuttx] Error 1
```
If I revert the following commit, it works again.
```
commit 1c2c0e4707a8e2e73378adcd0704ca8e2ef8e088 (tag: nuttx-20220128-212014)
Author: Xiang Xiao <xi...@xiaomi.com>
Date: Tue Jan 25 16:11:30 2022 +0800
arch/Toolchain.defs: Simplify the builtin library addition for EXTRA_LIBS
Signed-off-by: Xiang Xiao <xi...@xiaomi.com>
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #5332: arch/Toolchain.defs: Simplify addition builtin library to EXTRA_LIBS
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #5332:
URL: https://github.com/apache/incubator-nuttx/pull/5332#issuecomment-1021890450
> Changes works fine to me. The point that I was stating about involving linker via compiler is next. We can link program either with
>
> ```
> arm-none-eabi-ld --entry=main f1.o f2.o /usr/lib/gcc/arm-none-eabi/10.3.1/thumb/v7e-m+dp/hard/libm.a -o final.elf
> ```
>
> or
>
> ```
> arm-none-eabi-gcc -march=armv7e-m -mfpu=fpv5-d16 -mfloat-abi=hard --Xlinker --entry=main f1.o f2.o -lm -o final.elf
> ```
>
> those two calls are equivalent but in the second case the proper `libm.a` is tracked by the compiler automatically. We achieve the same by `arm-none-eabi-gcc -march=armv7e-m -mfpu=fpv5-d16 -mfloat-abi=hard --print-file-name=libm.a` and this is fine until we do not enable LTO. I've spent a lot of time trying to bring-up stable LTO build with one of my projects and it really didn't work stable with linking via `arm-none-eabi-ld` because it requires tight cooperation between compiler and linker. Until we do not need to enable LTO we are fine with current approach.
Yes, we have discussed the same issue before, and even make the similar change, but revert it later:(. I am still seeking the solution to make the mainline LTO in the ready state.
https://github.com/apache/incubator-nuttx/pull/3836
https://github.com/apache/incubator-nuttx/pull/3900
https://github.com/apache/incubator-nuttx/pull/4451
https://github.com/apache/incubator-nuttx/pull/4477
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko merged pull request #5332: arch/Toolchain.defs: Simplify addition builtin library to EXTRA_LIBS
Posted by GitBox <gi...@apache.org>.
pkarashchenko merged pull request #5332:
URL: https://github.com/apache/incubator-nuttx/pull/5332
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] masayuki2009 commented on pull request #5332: arch/Toolchain.defs: Simplify addition builtin library to EXTRA_LIBS
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #5332:
URL: https://github.com/apache/incubator-nuttx/pull/5332#issuecomment-1024883813
@xiaoxiang781216
>@masayuki2009 Could you try #5371?
I will try the PR.
>Do you know why CI doesn't catch this error before merge?
No, but it should catch the errors.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #5332: arch/Toolchain.defs: Simplify addition builtin library to EXTRA_LIBS
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #5332:
URL: https://github.com/apache/incubator-nuttx/pull/5332#issuecomment-1024083283
@pkarashchenko it's ready for merging now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #5332: arch/Toolchain.defs: Simplify addition builtin library to EXTRA_LIBS
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on pull request #5332:
URL: https://github.com/apache/incubator-nuttx/pull/5332#issuecomment-1021919737
@xiaoxiang781216 I will explore the discussion and also start thinking how this can be done. Thank you!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5332: arch/Toolchain.defs: Simplify addition builtin library to EXTRA_LIBS
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5332:
URL: https://github.com/apache/incubator-nuttx/pull/5332#discussion_r792441416
##########
File path: arch/arm/src/arm/Toolchain.defs
##########
@@ -103,21 +103,12 @@ OBJDUMP = $(CROSSDEV)objdump
# Add the builtin library
-EXTRA_LIBS += -lgcc
-EXTRA_LIBPATHS += -L "${shell dirname "`$(CC) $(ARCHCPUFLAGS) --print-libgcc-file-name`"}"
+EXTRA_LIBS := ${shell $(CC) $(ARCHCPUFLAGS) -print-file-name=libgcc.a}
Review comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #5332: arch/Toolchain.defs: Simplify addition builtin library to EXTRA_LIBS
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on pull request #5332:
URL: https://github.com/apache/incubator-nuttx/pull/5332#issuecomment-1020945939
I think with such approach we are becoming link order dependent. But let's wait CI to finish
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #5332: arch/Toolchain.defs: Simplify addition builtin library to EXTRA_LIBS
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #5332:
URL: https://github.com/apache/incubator-nuttx/pull/5332#issuecomment-1024881579
> @xiaoxiang781216 I noticed that sama5d4-ek:ksh build failed. Could you confirm what is happening now?
>
Because ARCHCPUFLAGS overwrite by board's Make.defs:
https://github.com/apache/incubator-nuttx/blob/master/boards/arm/sama5/sama5d4-ek/scripts/Make.defs#L51
so the flags used to find the library in Toolchain.defs is different from the compiling.
Could you try https://github.com/apache/incubator-nuttx/pull/5371?
Do you know why CI doesn't catch this error before merge?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5332: arch/Toolchain.defs: Simplify addition builtin library to EXTRA_LIBS
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5332:
URL: https://github.com/apache/incubator-nuttx/pull/5332#discussion_r795353094
##########
File path: arch/arm/src/arm/Toolchain.defs
##########
@@ -103,21 +103,12 @@ OBJDUMP = $(CROSSDEV)objdump
# Add the builtin library
-EXTRA_LIBS += -lgcc
-EXTRA_LIBPATHS += -L "${shell dirname "`$(CC) $(ARCHCPUFLAGS) --print-libgcc-file-name`"}"
+EXTRA_LIBS := ${shell $(CC) $(ARCHCPUFLAGS) --print-file-name=libgcc.a}
Review comment:
I thought both are equal. Let me provide a patch fix it. Maybe, it's better to add clang to the ci build.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #5332: arch/Toolchain.defs: Simplify addition builtin library to EXTRA_LIBS
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #5332:
URL: https://github.com/apache/incubator-nuttx/pull/5332#issuecomment-1022119712
> @xiaoxiang781216 I was going to approve and merge this PR, but see that now it is moved to "Draft". Do you expect any other changes in scope of this PR?
Please wait a moment, I am trying to enable clang with this change. Once the verification pass, I will remove the draft flag.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #5332: arch/Toolchain.defs: Simplify addition builtin library to EXTRA_LIBS
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on pull request #5332:
URL: https://github.com/apache/incubator-nuttx/pull/5332#issuecomment-1020945939
I think with such approach we are becoming link order dependent. But let's wait CI to finish
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #5332: arch/Toolchain.defs: Simplify addition builtin library to EXTRA_LIBS
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #5332:
URL: https://github.com/apache/incubator-nuttx/pull/5332#discussion_r795362533
##########
File path: arch/arm/src/arm/Toolchain.defs
##########
@@ -103,21 +103,12 @@ OBJDUMP = $(CROSSDEV)objdump
# Add the builtin library
-EXTRA_LIBS += -lgcc
-EXTRA_LIBPATHS += -L "${shell dirname "`$(CC) $(ARCHCPUFLAGS) --print-libgcc-file-name`"}"
+EXTRA_LIBS := ${shell $(CC) $(ARCHCPUFLAGS) --print-file-name=libgcc.a}
Review comment:
FYI, https://reviews.llvm.org/D25338
as far as i know, the situation has not been changed since then.
> Maybe, it's better to add clang to the ci build.
i agree.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #5332: arch/Toolchain.defs: Simplify addition builtin library to EXTRA_LIBS
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #5332:
URL: https://github.com/apache/incubator-nuttx/pull/5332#discussion_r795339169
##########
File path: arch/arm/src/arm/Toolchain.defs
##########
@@ -103,21 +103,12 @@ OBJDUMP = $(CROSSDEV)objdump
# Add the builtin library
-EXTRA_LIBS += -lgcc
-EXTRA_LIBPATHS += -L "${shell dirname "`$(CC) $(ARCHCPUFLAGS) --print-libgcc-file-name`"}"
+EXTRA_LIBS := ${shell $(CC) $(ARCHCPUFLAGS) --print-file-name=libgcc.a}
Review comment:
what's the motivation to change --print-libgcc-file-name to --print-file-name=libgcc.a?
iirc, the former has a better chance to work for clang configured to use non-libgcc runtime. (eg. compiler_rt)
i don't know if it matters here though.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5332: arch/Toolchain.defs: Simplify addition builtin library to EXTRA_LIBS
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5332:
URL: https://github.com/apache/incubator-nuttx/pull/5332#discussion_r795392174
##########
File path: arch/arm/src/arm/Toolchain.defs
##########
@@ -103,21 +103,12 @@ OBJDUMP = $(CROSSDEV)objdump
# Add the builtin library
-EXTRA_LIBS += -lgcc
-EXTRA_LIBPATHS += -L "${shell dirname "`$(CC) $(ARCHCPUFLAGS) --print-libgcc-file-name`"}"
+EXTRA_LIBS := ${shell $(CC) $(ARCHCPUFLAGS) --print-file-name=libgcc.a}
Review comment:
Fixed here: https://github.com/apache/incubator-nuttx/pull/5385
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #5332: arch/Toolchain.defs: Simplify addition builtin library to EXTRA_LIBS
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #5332:
URL: https://github.com/apache/incubator-nuttx/pull/5332#issuecomment-1021507148
@pkarashchenko the CI pass.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #5332: arch/Toolchain.defs: Simplify addition builtin library to EXTRA_LIBS
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on pull request #5332:
URL: https://github.com/apache/incubator-nuttx/pull/5332#issuecomment-1022090839
@xiaoxiang781216 I was going to approve and merge this PR, but see that now it is moved to "Draft". Do you expect any other changes in scope of this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #5332: arch/Toolchain.defs: Simplify addition builtin library to EXTRA_LIBS
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #5332:
URL: https://github.com/apache/incubator-nuttx/pull/5332#issuecomment-1021413933
> I think with such approach we are becoming link order dependent. But let's wait CI to finish
> IMO the only correct way to handle this is to involve linker via compiler using `-Xlinker` option to pass link flags. In this way the architecture is already defined and we do not need all this `--print-file-name` stuff. But we are far away from that
Could you give more info about these two problem? I can't catch the key issue you mention.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5332: arch/Toolchain.defs: Simplify addition builtin library to EXTRA_LIBS
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5332:
URL: https://github.com/apache/incubator-nuttx/pull/5332#discussion_r792311962
##########
File path: arch/arm/src/arm/Toolchain.defs
##########
@@ -103,21 +103,12 @@ OBJDUMP = $(CROSSDEV)objdump
# Add the builtin library
-EXTRA_LIBS += -lgcc
-EXTRA_LIBPATHS += -L "${shell dirname "`$(CC) $(ARCHCPUFLAGS) --print-libgcc-file-name`"}"
+EXTRA_LIBS := ${shell $(CC) $(ARCHCPUFLAGS) -print-file-name=libgcc.a}
Review comment:
```suggestion
EXTRA_LIBS := ${shell $(CC) $(ARCHCPUFLAGS) --print-libgcc-file-name}
```
or if you want common style
```suggestion
EXTRA_LIBS := ${shell $(CC) $(ARCHCPUFLAGS) --print-file-name=libgcc.a}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #5332: arch/Toolchain.defs: Simplify addition builtin library to EXTRA_LIBS
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #5332:
URL: https://github.com/apache/incubator-nuttx/pull/5332#issuecomment-1024881579
> @xiaoxiang781216 I noticed that sama5d4-ek:ksh build failed. Could you confirm what is happening now?
>
Because ARCHCPUFLAGS overwrite by board's Make.defs:
https://github.com/apache/incubator-nuttx/blob/master/boards/arm/sama5/sama5d4-ek/scripts/Make.defs#L51
so the flags used to find the library in Toolchain.defs is different from the compiling.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5332: arch/Toolchain.defs: Simplify addition builtin library to EXTRA_LIBS
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5332:
URL: https://github.com/apache/incubator-nuttx/pull/5332#discussion_r792282163
##########
File path: arch/arm/src/arm/Toolchain.defs
##########
@@ -103,21 +103,12 @@ OBJDUMP = $(CROSSDEV)objdump
# Add the builtin library
-EXTRA_LIBS += -lgcc
-EXTRA_LIBPATHS += -L "${shell dirname "`$(CC) $(ARCHCPUFLAGS) --print-libgcc-file-name`"}"
+EXTRA_LIBS := ${shell $(CC) $(ARCHCPUFLAGS) -print-file-name=libgcc.a}
ifneq ($(CONFIG_LIBM),y)
- LIBM_PATH = "${shell dirname "`$(CC) $(ARCHCPUFLAGS) --print-file-name=libm.a`"}"
-
- # Check if libm is provided by the compiler
-
- ifneq ($(LIBM_PATH),".")
- EXTRA_LIBS += -lm
- EXTRA_LIBPATHS += -L $(LIBM_PATH)
- endif
+ EXTRA_LIBS += ${wildcard ${shell $(CC) $(ARCHCPUFLAGS) --print-file-name=libm.a}}
Review comment:
Two reason:
1. CONFIG_LIBM isn't enabled by default, so libm.a will be searched by default.
2. But the toolchain doesn't contain libm.a library
Here is the related change: https://github.com/apache/incubator-nuttx/pull/5309
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5332: arch/Toolchain.defs: Simplify addition builtin library to EXTRA_LIBS
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5332:
URL: https://github.com/apache/incubator-nuttx/pull/5332#discussion_r792282163
##########
File path: arch/arm/src/arm/Toolchain.defs
##########
@@ -103,21 +103,12 @@ OBJDUMP = $(CROSSDEV)objdump
# Add the builtin library
-EXTRA_LIBS += -lgcc
-EXTRA_LIBPATHS += -L "${shell dirname "`$(CC) $(ARCHCPUFLAGS) --print-libgcc-file-name`"}"
+EXTRA_LIBS := ${shell $(CC) $(ARCHCPUFLAGS) -print-file-name=libgcc.a}
ifneq ($(CONFIG_LIBM),y)
- LIBM_PATH = "${shell dirname "`$(CC) $(ARCHCPUFLAGS) --print-file-name=libm.a`"}"
-
- # Check if libm is provided by the compiler
-
- ifneq ($(LIBM_PATH),".")
- EXTRA_LIBS += -lm
- EXTRA_LIBPATHS += -L $(LIBM_PATH)
- endif
+ EXTRA_LIBS += ${wildcard ${shell $(CC) $(ARCHCPUFLAGS) --print-file-name=libm.a}}
Review comment:
Two reason:
1. CONFIG_LIBM isn't enabled by default, so libm.a will be searched by default.
2. But some toolchain mayn't contain libm.a library
Here is the related change: https://github.com/apache/incubator-nuttx/pull/5309
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5332: arch/Toolchain.defs: Simplify addition builtin library to EXTRA_LIBS
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5332:
URL: https://github.com/apache/incubator-nuttx/pull/5332#discussion_r795353094
##########
File path: arch/arm/src/arm/Toolchain.defs
##########
@@ -103,21 +103,12 @@ OBJDUMP = $(CROSSDEV)objdump
# Add the builtin library
-EXTRA_LIBS += -lgcc
-EXTRA_LIBPATHS += -L "${shell dirname "`$(CC) $(ARCHCPUFLAGS) --print-libgcc-file-name`"}"
+EXTRA_LIBS := ${shell $(CC) $(ARCHCPUFLAGS) --print-file-name=libgcc.a}
Review comment:
I thought both are equal. Let me provide a patch fix it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org