casaroli commented on code in PR #19508: URL: https://github.com/apache/nuttx/pull/19508#discussion_r3637134099
########## arch/arm/src/common/Toolchain.defs: ########## @@ -584,6 +584,25 @@ CELFFLAGS = $(CFLAGS) -fvisibility=hidden -mlong-calls # --target1-abs CXXELFFLAGS = $(CXXFLAGS)-fvisibility=hidden -mlong-calls ifeq ($(CONFIG_PIC),y) + # NOTE: nearly every board Make.defs includes this file and then assigns + # + # CFLAGS := $(ARCHCFLAGS) $(ARCHOPTIMIZATION) $(ARCHCPUFLAGS) ... + # + # with ':=', which silently discards this flag. A board enabling + # CONFIG_PIC has to re-apply it after that assignment, and then filter it + # back out of CPICFLAGS and CELFFLAGS, because GCC rejects --fixed-r10 + # alongside the -mpic-register=r10 those carry. See + # boards/arm/mps/mps3-an547/scripts/Make.defs for the pattern. + # + # The flag cannot simply move to ARCHCPUFLAGS to survive the ':=', because + # the module flags derive from CFLAGS and would then hit exactly that + # rejected combination. + # + # Losing it is silent and the symptom is remote from the cause: the build + # succeeds, and only a callback from base firmware into module code + # misbehaves, reading its data through a register the firmware has since + # felt free to reuse. + CFLAGS += --fixed-r10 Review Comment: I agree that your suggestio is the correct fix, however we need to change all 500 arm files, so that `ARCHPICFLAGS` and `CPICFLAGS` all move to Toolchain.defs This is a huge change, so I have prepared the fix in a separate PR so you can analyze and compare, and we can check the CI status: https://github.com/apache/nuttx/pull/19510 ########## arch/arm/src/common/Toolchain.defs: ########## @@ -584,6 +584,25 @@ CELFFLAGS = $(CFLAGS) -fvisibility=hidden -mlong-calls # --target1-abs CXXELFFLAGS = $(CXXFLAGS)-fvisibility=hidden -mlong-calls ifeq ($(CONFIG_PIC),y) + # NOTE: nearly every board Make.defs includes this file and then assigns + # + # CFLAGS := $(ARCHCFLAGS) $(ARCHOPTIMIZATION) $(ARCHCPUFLAGS) ... + # + # with ':=', which silently discards this flag. A board enabling + # CONFIG_PIC has to re-apply it after that assignment, and then filter it + # back out of CPICFLAGS and CELFFLAGS, because GCC rejects --fixed-r10 + # alongside the -mpic-register=r10 those carry. See + # boards/arm/mps/mps3-an547/scripts/Make.defs for the pattern. + # + # The flag cannot simply move to ARCHCPUFLAGS to survive the ':=', because + # the module flags derive from CFLAGS and would then hit exactly that + # rejected combination. + # + # Losing it is silent and the symptom is remote from the cause: the build + # succeeds, and only a callback from base firmware into module code + # misbehaves, reading its data through a register the firmware has since + # felt free to reuse. + CFLAGS += --fixed-r10 Review Comment: I agree that your suggestion is the correct fix, however we need to change all 500 arm files, so that `ARCHPICFLAGS` and `CPICFLAGS` all move to Toolchain.defs This is a huge change, so I have prepared the fix in a separate PR so you can analyze and compare, and we can check the CI status: https://github.com/apache/nuttx/pull/19510 -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
