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 2021/06/14 07:08:33 UTC

[GitHub] [incubator-nuttx] yamt opened a new pull request #3900: Revert "Make: use gcc as LD"

yamt opened a new pull request #3900:
URL: https://github.com/apache/incubator-nuttx/pull/3900


   ## Summary
   
   This reverts commit 45672c269db13f59bdaa417e564837e8bbb6c8c1.
   
   Because:
   
   * It's very confusing to have cc as LD.
   * I don't see what "-nostartfiles -nodefaultlibs" in LDFLAGS are
     supposed to do when we use LD directly. It would be simpler to
     remove them from our LDFLAGS.
   
   ## Impact
   
   ## Testing
   
   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #3900: Revert "Make: use gcc as LD"

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #3900:
URL: https://github.com/apache/incubator-nuttx/pull/3900#issuecomment-873848204


   I am fine with either approach, but could you incorporate your LTO demo into mainline? Otherwise, the same change may be created again for LTO.


-- 
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 edited a comment on pull request #3900: Revert "Make: use gcc as LD"

Posted by GitBox <gi...@apache.org>.
yamt edited a comment on pull request #3900:
URL: https://github.com/apache/incubator-nuttx/pull/3900#issuecomment-860358955


   i played a bit with LTO.
   https://github.com/yamt/garbage/tree/master/lto
   for me, it seems working with ld.
   
   also, it seems gcc folks are thinking ld should work.
   http://gcc.gnu.org/wiki/whopr/driver
   
   my impression is that it isn't a good idea to use LD=cc for the LTO purpose either.


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] Ouss4 commented on pull request #3900: Revert "Make: use gcc as LD"

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #3900:
URL: https://github.com/apache/incubator-nuttx/pull/3900#issuecomment-883988063


   @xiaoxiang781216 @gustavonihei @AlexanderVasiljev are we going to have more PRs like #4173?  Otherwise we should merge this one, it's been sitting here for a while.


-- 
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] acassis edited a comment on pull request #3900: Revert "Make: use gcc as LD"

Posted by GitBox <gi...@apache.org>.
acassis edited a comment on pull request #3900:
URL: https://github.com/apache/incubator-nuttx/pull/3900#issuecomment-917477711


   I thought about the idea of going ahead and removing "-nostartfiles -nodefaultlibs" from all LDFLAGS, but we cannot do it without double check each toolchain. For example:
   ```
   # Pinguino toolchain under Linux
   
   ifeq ($(CONFIG_MIPS32_TOOLCHAIN),PINGUINOL)
   ```
   
   This is a toolchain that use to be very outdated and removing those flags could bring unexpected results.


-- 
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] gustavonihei edited a comment on pull request #3900: Revert "Make: use gcc as LD"

Posted by GitBox <gi...@apache.org>.
gustavonihei edited a comment on pull request #3900:
URL: https://github.com/apache/incubator-nuttx/pull/3900#issuecomment-860710201


   > 
   > * iirc, linux supports LTO with ld. can you explain why it doesn't work for us?
   
   Linux Kernel support LTO with Clang-only so far, and using the LLD, which is the LLVM linker.
   
   > also, it seems gcc folks are thinking ld should work.
   > http://gcc.gnu.org/wiki/whopr/driver
   
   I believe that wiki page might not be up-to-date, since it is already a bit old. Is there a specific detail from this guide you'd like to bring to the discussion?
   If you look at the documentation for the '-flto' flag, it is clearly stated that:
   
   > _The important thing to keep in mind is that to enable link-time optimizations you need to use the GCC driver to perform the link step_
   
   https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html#:~:text=the%20important%20thing%20to%20keep%20in%20mind%20is%20that%20to%20enable%20link-time%20optimizations%20you%20need%20to%20use%20the%20gcc%20driver%20to%20perform%20the%20link%20step.
   
   > * anyway, LTO thing is a bit off-topic. the change in question was to fix #3826 and my point is that it doesn't seem like an appropriate fix.
   
   I understand that a future support for LTO should not be the sole reason for that decision, and I agree that removing `-nostarfiles` and `-nodefaultlibs` is also a solution for the issue #3826.
   But I don't see why the implemented solution is not appropriate. It indeed fixes the issue and it has the advantage of setting up the build system for a future improvement.
   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] yamt commented on pull request #3900: Revert "Make: use gcc as LD"

Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #3900:
URL: https://github.com/apache/incubator-nuttx/pull/3900#issuecomment-860288672






-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] yamt commented on pull request #3900: Revert "Make: use gcc as LD"

Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #3900:
URL: https://github.com/apache/incubator-nuttx/pull/3900#issuecomment-874443249


   > I am fine with either approach, but could you incorporate your LTO demo into mainline? Otherwise, the same change may be created again for LTO.
   
   maybe i will try.
   however, honestly, i feel it's very unfair to require me to do the lto work while it was not and will not required for "gcc as ld" supporters. after all, lto is their reasoning, not mine.


-- 
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 pull request #3900: Revert "Make: use gcc as LD"

Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #3900:
URL: https://github.com/apache/incubator-nuttx/pull/3900#issuecomment-882551932


   btw, i updated my demo to use a library. it seems working for both of gcc and clang so far.
   https://github.com/yamt/garbage/commit/506efb8445aaa5a7abf6879873b0b5d701a3cab6


-- 
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] hartmannathan commented on pull request #3900: Revert "Make: use gcc as LD"

Posted by GitBox <gi...@apache.org>.
hartmannathan commented on pull request #3900:
URL: https://github.com/apache/incubator-nuttx/pull/3900#issuecomment-917530912


   On Sat, Sep 11, 2021 at 4:31 PM Alan Carvalho de Assis <
   ***@***.***> wrote:
   
   > @yamt <https://github.com/yamt> @xiaoxiang781216
   > <https://github.com/xiaoxiang781216> please take a look, this Revert
   > brought back an old issue, now all ARM and MIPS boards are breaking because
   > of it:
   >
   > LDFLAGS += -nostartfiles ...
   >
   > "arm-none-eabi-ld: Error: unable to disambiguate: -nostartfiles (did you
   > mean --nostartfiles ?)"
   >
   > As @hartmannathan <https://github.com/hartmannathan> reported here #3209
   > <https://github.com/apache/incubator-nuttx/issues/3209> it will happen if
   > "Binutils is updated to 2.36.x". So probably our CI is using an old version
   > and the problem doesn't show up.
   >
   > —
   > You are receiving this because you were mentioned.
   >
   >
   > Reply to this email directly, view it on GitHub
   > <https://github.com/apache/incubator-nuttx/pull/3900#issuecomment-917475760>,
   > or unsubscribe
   > <https://github.com/notifications/unsubscribe-auth/AOD4O5ZXWNJNAKYGDKDMUW3UBO4BDANCNFSM46PSFL7A>
   > .
   > Triage notifications on the go with GitHub Mobile for iOS
   > <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
   > or Android
   > <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
   >
   >
   
   In the CWIKI, I wrote a more detailed explanation:
   
   https://cwiki.apache.org/confluence/display/NUTTX/NuttX+10.2?desktop=true
   ¯oName=markdown#ld-now-called-through-gcc
   


-- 
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 pull request #3900: Revert "Make: use gcc as LD"

Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #3900:
URL: https://github.com/apache/incubator-nuttx/pull/3900#issuecomment-873764196


   @xiaoxiang781216 @gustavonihei 
   the discussion seemed settled. can you consider merging this? or are you working on alternatives?


-- 
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 edited a comment on pull request #3900: Revert "Make: use gcc as LD"

Posted by GitBox <gi...@apache.org>.
yamt edited a comment on pull request #3900:
URL: https://github.com/apache/incubator-nuttx/pull/3900#issuecomment-882527566


   @xiaoxiang781216 @gustavonihei 
   
   i haven't found a chance to investigate the lto things.
   you haven't either, it guess.
   on the other hand, the "cc as LD" thing seems still causing issues. https://github.com/apache/incubator-nuttx/pull/4173
   how about merging this revert for 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] gustavonihei edited a comment on pull request #3900: Revert "Make: use gcc as LD"

Posted by GitBox <gi...@apache.org>.
gustavonihei edited a comment on pull request #3900:
URL: https://github.com/apache/incubator-nuttx/pull/3900#issuecomment-860710201


   > 
   > * iirc, linux supports LTO with ld. can you explain why it doesn't work for us?
   
   Linux Kernel support LTO with Clang-only so far, and using the LLD, which is the LLVM linker.
   
   > also, it seems gcc folks are thinking ld should work.
   > http://gcc.gnu.org/wiki/whopr/driver
   
   I believe that wiki page might not be up-to-date, since it is already a bit old. Is there a specific detail from this guide you'd like to bring to the discussion?
   If you look at the documentation for the `-flto` flag, it is clearly stated that:
   
   > _The important thing to keep in mind is that to enable link-time optimizations you need to use the GCC driver to perform the link step_
   
   https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html#:~:text=the%20important%20thing%20to%20keep%20in%20mind%20is%20that%20to%20enable%20link-time%20optimizations%20you%20need%20to%20use%20the%20gcc%20driver%20to%20perform%20the%20link%20step.
   
   > * anyway, LTO thing is a bit off-topic. the change in question was to fix #3826 and my point is that it doesn't seem like an appropriate fix.
   
   I understand that a future support for LTO should not be the sole reason for that decision, and I agree that removing `-nostarfiles` and `-nodefaultlibs` is also a solution for the issue #3826.
   But I don't see why the implemented solution is not appropriate. It indeed fixes the issue and it has the advantage of setting up the build system for a future improvement.
   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] hartmannathan commented on pull request #3900: Revert "Make: use gcc as LD"

Posted by GitBox <gi...@apache.org>.
hartmannathan commented on pull request #3900:
URL: https://github.com/apache/incubator-nuttx/pull/3900#issuecomment-859650629


   > > Because:
   > > 
   > > * It's very confusing to have cc as LD.
   > 
   > Yes, it's a little bit confusing, but if you want to enable LTO, you must use gcc to drive the link process as far as I know. If we can't find a method to let ld work with LTO, it's better to keep the change as it.
   > 
   > > * I don't see what "-nostartfiles -nodefaultlibs" in LDFLAGS are
   > >   supposed to do when we use LD directly. It would be simpler to
   > >   remove them from our LDFLAGS.
   
   Agreed that Link Time Optimization would be a good thing to make a reality, if possible. We may run into some additional fallout from the 45672c2 change, like what happened with clang (fixed in #3904). But so far it looks good.


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] yamt edited a comment on pull request #3900: Revert "Make: use gcc as LD"

Posted by GitBox <gi...@apache.org>.
yamt edited a comment on pull request #3900:
URL: https://github.com/apache/incubator-nuttx/pull/3900#issuecomment-882527566


   @xiaoxiang781216 @gustavonihei 
   
   i haven't found a chance to investigate the lto things.
   you haven't either, it guess.
   on the other hand, the "cc as LD" thing seems still causing issues. https://github.com/apache/incubator-nuttx/pull/4173
   how about merging this revert for 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] yamt commented on pull request #3900: Revert "Make: use gcc as LD"

Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #3900:
URL: https://github.com/apache/incubator-nuttx/pull/3900#issuecomment-882527566






-- 
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 merged pull request #3900: Revert "Make: use gcc as LD"

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #3900:
URL: https://github.com/apache/incubator-nuttx/pull/3900


   


-- 
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] acassis commented on pull request #3900: Revert "Make: use gcc as LD"

Posted by GitBox <gi...@apache.org>.
acassis commented on pull request #3900:
URL: https://github.com/apache/incubator-nuttx/pull/3900#issuecomment-917475760


   @yamt @xiaoxiang781216  please take a look, this Revert brought back an old issue, now all ARM and MIPS boards are breaking because of it:
   ```
   LDFLAGS += -nostartfiles ...
   ```
   
   "arm-none-eabi-ld: Error: unable to disambiguate: -nostartfiles (did you mean --nostartfiles ?)"
   
   As @hartmannathan reported here https://github.com/apache/incubator-nuttx/issues/3209 it will happen if "Binutils is updated to  2.36.x". So probably our CI is using an old version and the problem doesn't show up.


-- 
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 pull request #3900: Revert "Make: use gcc as LD"

Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #3900:
URL: https://github.com/apache/incubator-nuttx/pull/3900#issuecomment-861044754


   > > * iirc, linux supports LTO with ld. can you explain why it doesn't work for us?
   > 
   > Linux Kernel support LTO with Clang-only so far, and using the LLD, which is the LLVM linker.
   
   yes.
   
   > 
   > > also, it seems gcc folks are thinking ld should work.
   > > http://gcc.gnu.org/wiki/whopr/driver
   > 
   > I believe that wiki page might not be up-to-date, since it is already a bit old. Is there a specific detail from this guide you'd like to bring to the discussion?
   > If you look at the documentation for the `-flto` flag, it is clearly stated that:
   > 
   > > _The important thing to keep in mind is that to enable link-time optimizations you need to use the GCC driver to perform the link step_
   > 
   > https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html#:~:text=the%20important%20thing%20to%20keep%20in%20mind%20is%20that%20to%20enable%20link-time%20optimizations%20you%20need%20to%20use%20the%20gcc%20driver%20to%20perform%20the%20link%20step.
   
   have you looked at the example i provided?
   https://github.com/yamt/garbage/tree/master/lto
   it includes gcc. to me it looks working as documented in the wiki.
   
   > 
   > > * anyway, LTO thing is a bit off-topic. the change in question was to fix #3826 and my point is that it doesn't seem like an appropriate fix.
   > 
   > I understand that a future support for LTO should not be the sole reason for that decision, and I agree that removing `-nostarfiles` and `-nodefaultlibs` is also a solution for the issue #3826.
   > But I don't see why the implemented solution is not appropriate. It indeed fixes the issue and it has the advantage of setting up the build system for a future improvement.
   
   * as i stated, using cc as LD is very confusing. if you really want to use cc for linking, it's better to use CC. in some places we already do that. (grep CCLINKFLAG)
   * do you have a POC patch to enable LTO that way? otherwise the "setting up the build system for a future improvement" sounds moot.
   
   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] acassis commented on pull request #3900: Revert "Make: use gcc as LD"

Posted by GitBox <gi...@apache.org>.
acassis commented on pull request #3900:
URL: https://github.com/apache/incubator-nuttx/pull/3900#issuecomment-917477711


   I thought about the idea of going ahead and removing "-nostartfiles -nodefaultlibs" from all LDFLAGS, but we cannot do it without double check each toolchain. For example:
   ```
   # Pinguino toolchain under Linux
   
   ifeq ($(CONFIG_MIPS32_TOOLCHAIN),PINGUINOL)
   ```
   
   This is a toolchain that use to be very outdated and remove those flags could bring unexpected results.


-- 
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] juniskane commented on pull request #3900: Revert "Make: use gcc as LD"

Posted by GitBox <gi...@apache.org>.
juniskane commented on pull request #3900:
URL: https://github.com/apache/incubator-nuttx/pull/3900#issuecomment-861412208


   I have no opinion if this should be reverted or not, but doing changes like this repeatedly is hard for commercial users because all  out-of-tree board Make.defs files needs to be modified every single time when something like this is either applied or reverted.
   
   At least the -Wl, should have been expanded from make variable, say $(LDFLAGPREFIX), so that changing between gcc and ld does not need to touch so many files again.


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] yamt commented on pull request #3900: Revert "Make: use gcc as LD"

Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #3900:
URL: https://github.com/apache/incubator-nuttx/pull/3900#issuecomment-874443249


   > I am fine with either approach, but could you incorporate your LTO demo into mainline? Otherwise, the same change may be created again for LTO.
   
   maybe i will try.
   however, honestly, i feel it's very unfair to require me to do the lto work while it was not and will not required for "gcc as ld" supporters. after all, lto is their reasoning, not mine.


-- 
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 edited a comment on pull request #3900: Revert "Make: use gcc as LD"

Posted by GitBox <gi...@apache.org>.
yamt edited a comment on pull request #3900:
URL: https://github.com/apache/incubator-nuttx/pull/3900#issuecomment-882527566


   @xiaoxiang781216 @gustavonihei 
   
   i haven't found a chance to investigate the lto things.
   you haven't either, it guess.
   on the other hand, the "cc as LD" thing seems still causing issues. https://github.com/apache/incubator-nuttx/pull/4173
   how about merging this revert for 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] xiaoxiang781216 edited a comment on pull request #3900: Revert "Make: use gcc as LD"

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #3900:
URL: https://github.com/apache/incubator-nuttx/pull/3900#issuecomment-913953478


   @gustavonihei and @Ouss4 how about we merge this PR? because using gcc as ld doesn't work well with clang toolchain. I can provide lto demo with ld after this PR 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 pull request #3900: Revert "Make: use gcc as LD"

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #3900:
URL: https://github.com/apache/incubator-nuttx/pull/3900#issuecomment-859552972


   > Because:
   > 
   > * It's very confusing to have cc as LD.
   
   Yes, but if you want to enable LTO, you must use gcc to drive the link process as far as I know. If we can't find a method to let ld work with LTO, it's better to keep the change as it.
   
   > * I don't see what "-nostartfiles -nodefaultlibs" in LDFLAGS are
   >   supposed to do when we use LD directly. It would be simpler to
   >   remove them from our LDFLAGS.
   > 
   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #3900: Revert "Make: use gcc as LD"

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #3900:
URL: https://github.com/apache/incubator-nuttx/pull/3900#issuecomment-875422532


   @gustavonihei was your opinion?


-- 
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 pull request #3900: Revert "Make: use gcc as LD"

Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #3900:
URL: https://github.com/apache/incubator-nuttx/pull/3900#issuecomment-882527566


   the "cc as LD<https://github.com/apache/incubator-nuttx/pull/4173


-- 
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] gustavonihei commented on pull request #3900: Revert "Make: use gcc as LD"

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on pull request #3900:
URL: https://github.com/apache/incubator-nuttx/pull/3900#issuecomment-860710201


   > 
   > * iirc, linux supports LTO with ld. can you explain why it doesn't work for us?
   
   Linux Kernel support LTO with Clang-only so far, and using the LLD, which is the LLVM linker.
   
   > also, it seems gcc folks are thinking ld should work.
   > http://gcc.gnu.org/wiki/whopr/driver
   
   This wiki page is quite old and might not be up-to-date. Is there a specific detail from this guide you'd like to bring to the discussion?
   If you look at the documentation for the '-flto' flag, it is clearly stated that:
   
   > _The important thing to keep in mind is that to enable link-time optimizations you need to use the GCC driver to perform the link step_
   
   https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html#:~:text=the%20important%20thing%20to%20keep%20in%20mind%20is%20that%20to%20enable%20link-time%20optimizations%20you%20need%20to%20use%20the%20gcc%20driver%20to%20perform%20the%20link%20step.
   
   > * anyway, LTO thing is a bit off-topic. the change in question was to fix #3826 and my point is that it doesn't seem like an appropriate fix.
   
   I understand that a future support for LTO should not be the sole reason for that decision, and I agree that removing `-nostarfiles` and `-nodefaultlibs` is also a solution for the issue #3826.
   But I don't see why the implemented solution is not appropriate. It indeed fixes the issue and it has the advantage of setting up the build system for a future improvement.
   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] yamt commented on pull request #3900: Revert "Make: use gcc as LD"

Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #3900:
URL: https://github.com/apache/incubator-nuttx/pull/3900#issuecomment-882527566






-- 
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 #3900: Revert "Make: use gcc as LD"

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #3900:
URL: https://github.com/apache/incubator-nuttx/pull/3900#issuecomment-913953478


   @gustavonihei and @Ouss4 how about we merge this PR? because using gcc as ld doesn't work well with clang toolchain.


-- 
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] gustavonihei commented on pull request #3900: Revert "Make: use gcc as LD"

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on pull request #3900:
URL: https://github.com/apache/incubator-nuttx/pull/3900#issuecomment-875805030


   > @gustavonihei was your opinion?
   
   My opinion is somewhat like yours, I am fine either way.
   I am more inclined to simply drop this PR. I don’t agree with the “confuse” reasoning. But I won’t oppose to the decision of merging it also.


-- 
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 #3900: Revert "Make: use gcc as LD"

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #3900:
URL: https://github.com/apache/incubator-nuttx/pull/3900#issuecomment-859552972


   > Because:
   > 
   > * It's very confusing to have cc as LD.
   
   Yes, it's a little bit confusing, but if you want to enable LTO, you must use gcc to drive the link process as far as I know. If we can't find a method to let ld work with LTO, it's better to keep the change as it.
   
   > * I don't see what "-nostartfiles -nodefaultlibs" in LDFLAGS are
   >   supposed to do when we use LD directly. It would be simpler to
   >   remove them from our LDFLAGS.
   > 
   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org