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/19 18:00:50 UTC

[GitHub] [incubator-nuttx] xiaoxiang781216 opened a new pull request #5282: Update board Make.defs

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


   ## Summary
   
   - board/bl602: Remove -fshort-enums 
   - board: Remove -fstrict-volatile-bitfields 
   - board: Remove -fno-omit-frame-pointer from Make.defs
   
   ## Impact
   Unify the config in all Make.defs
   
   ## 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.

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 #5282: Update board Make.defs

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5282:
URL: https://github.com/apache/incubator-nuttx/pull/5282#discussion_r788764505



##########
File path: boards/risc-v/bl602/bl602evb/scripts/Make.defs
##########
@@ -45,8 +45,8 @@ ifneq ($(CONFIG_DEBUG_NOOPT),y)
 endif
 
 ARCHCPUFLAGS += -mno-relax
-ARCHCFLAGS = -fno-builtin -ffunction-sections -fdata-sections -fstrict-volatile-bitfields -fshort-enums -fno-omit-frame-pointer
-ARCHCXXFLAGS = -fno-builtin -ffunction-sections -fdata-sections -fstrict-volatile-bitfields -fshort-enums -fno-omit-frame-pointer -fno-exceptions -fcheck-new -std=c++17 -D__NuttX__ -pipe -nostdinc++
+ARCHCFLAGS = -fno-builtin -ffunction-sections -fdata-sections

Review comment:
       General question on `-fshort-enums`. Shouldn't this give better code size?
   Do we have a Kconfig options to specify extra options like this? It might be needed if code is linked to third party pre-compiled libraries that are compiled with `-fshort-enums` and have pointers to enums or arrays of enums passed as parameters to functions.

##########
File path: boards/xtensa/esp32s2/esp32s2-saola-1/scripts/Make.defs
##########
@@ -54,7 +54,7 @@ ifneq ($(CONFIG_DEBUG_NOOPT),y)
   ARCHOPTIMIZATION += $(MAXOPTIMIZATION) -fno-strict-aliasing
 endif
 
-ARCHCFLAGS = -fno-builtin -ffunction-sections -fdata-sections -fstrict-volatile-bitfields -mlongcalls
+ARCHCFLAGS = -fno-builtin -ffunction-sections -fdata-sections -mlongcalls

Review comment:
       Do we need to add `ifeq ($(CONFIG_FRAME_POINTER),y)`?

##########
File path: boards/xtensa/esp32/esp32-wrover-kit/scripts/Make.defs
##########
@@ -73,7 +73,7 @@ ifneq ($(CONFIG_DEBUG_NOOPT),y)
   ARCHOPTIMIZATION += $(MAXOPTIMIZATION) -fno-strict-aliasing
 endif
 
-ARCHCFLAGS = -fno-builtin -ffunction-sections -fdata-sections -fstrict-volatile-bitfields -mlongcalls
+ARCHCFLAGS = -fno-builtin -ffunction-sections -fdata-sections -mlongcalls

Review comment:
       Do we need to add `ifeq ($(CONFIG_FRAME_POINTER),y)`?

##########
File path: boards/sparc/bm3823/xx3823/scripts/Make.defs
##########
@@ -55,7 +55,7 @@ ifeq ($(CONFIG_DEBUG_SYMBOLS),y)
 endif
 
 ifneq ($(CONFIG_DEBUG_NOOPT),y)
-  ARCHOPTIMIZATION += $(MAXOPTIMIZATION) -fno-strict-aliasing -fomit-frame-pointer
+  ARCHOPTIMIZATION += $(MAXOPTIMIZATION) -fno-strict-aliasing

Review comment:
       Do we need to add `ifeq ($(CONFIG_FRAME_POINTER),y)`?

##########
File path: boards/xtensa/esp32/esp32-devkitc/scripts/Make.defs
##########
@@ -73,7 +73,7 @@ ifneq ($(CONFIG_DEBUG_NOOPT),y)
   ARCHOPTIMIZATION += $(MAXOPTIMIZATION) -fno-strict-aliasing
 endif
 
-ARCHCFLAGS = -fno-builtin -ffunction-sections -fdata-sections -fstrict-volatile-bitfields -mlongcalls
+ARCHCFLAGS = -fno-builtin -ffunction-sections -fdata-sections -mlongcalls

Review comment:
       Do we need to add `ifeq ($(CONFIG_FRAME_POINTER),y)`?

##########
File path: boards/xtensa/esp32/ttgo_lora_esp32/scripts/Make.defs
##########
@@ -73,7 +73,7 @@ ifneq ($(CONFIG_DEBUG_NOOPT),y)
   ARCHOPTIMIZATION += $(MAXOPTIMIZATION) -fno-strict-aliasing
 endif
 
-ARCHCFLAGS = -fno-builtin -ffunction-sections -fdata-sections -fstrict-volatile-bitfields -mlongcalls
+ARCHCFLAGS = -fno-builtin -ffunction-sections -fdata-sections -mlongcalls

Review comment:
       Do we need to add `ifeq ($(CONFIG_FRAME_POINTER),y)`?

##########
File path: boards/xtensa/esp32/esp32-ethernet-kit/scripts/Make.defs
##########
@@ -73,7 +73,7 @@ ifneq ($(CONFIG_DEBUG_NOOPT),y)
   ARCHOPTIMIZATION += $(MAXOPTIMIZATION) -fno-strict-aliasing
 endif
 
-ARCHCFLAGS = -fno-builtin -ffunction-sections -fdata-sections -fstrict-volatile-bitfields -mlongcalls
+ARCHCFLAGS = -fno-builtin -ffunction-sections -fdata-sections -mlongcalls

Review comment:
       Do we need to add `ifeq ($(CONFIG_FRAME_POINTER),y)`?

##########
File path: boards/sparc/bm3803/xx3803/scripts/Make.defs
##########
@@ -55,7 +55,7 @@ ifeq ($(CONFIG_DEBUG_SYMBOLS),y)
 endif
 
 ifneq ($(CONFIG_DEBUG_NOOPT),y)
-  ARCHOPTIMIZATION += $(MAXOPTIMIZATION) -fno-strict-aliasing -fomit-frame-pointer
+  ARCHOPTIMIZATION += $(MAXOPTIMIZATION) -fno-strict-aliasing

Review comment:
       Do we need to add `ifeq ($(CONFIG_FRAME_POINTER),y)` case here?




-- 
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 #5282: Update board Make.defs

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5282:
URL: https://github.com/apache/incubator-nuttx/pull/5282#discussion_r788911816



##########
File path: boards/risc-v/bl602/bl602evb/scripts/Make.defs
##########
@@ -45,8 +45,8 @@ ifneq ($(CONFIG_DEBUG_NOOPT),y)
 endif
 
 ARCHCPUFLAGS += -mno-relax
-ARCHCFLAGS = -fno-builtin -ffunction-sections -fdata-sections -fstrict-volatile-bitfields -fshort-enums -fno-omit-frame-pointer
-ARCHCXXFLAGS = -fno-builtin -ffunction-sections -fdata-sections -fstrict-volatile-bitfields -fshort-enums -fno-omit-frame-pointer -fno-exceptions -fcheck-new -std=c++17 -D__NuttX__ -pipe -nostdinc++
+ARCHCFLAGS = -fno-builtin -ffunction-sections -fdata-sections

Review comment:
       Probably we need to check ROM API calls if those have enum arrays as arguments or return values.




-- 
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 #5282: Update board Make.defs

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5282:
URL: https://github.com/apache/incubator-nuttx/pull/5282#discussion_r788905969



##########
File path: boards/risc-v/bl602/bl602evb/scripts/Make.defs
##########
@@ -45,8 +45,8 @@ ifneq ($(CONFIG_DEBUG_NOOPT),y)
 endif
 
 ARCHCPUFLAGS += -mno-relax
-ARCHCFLAGS = -fno-builtin -ffunction-sections -fdata-sections -fstrict-volatile-bitfields -fshort-enums -fno-omit-frame-pointer
-ARCHCXXFLAGS = -fno-builtin -ffunction-sections -fdata-sections -fstrict-volatile-bitfields -fshort-enums -fno-omit-frame-pointer -fno-exceptions -fcheck-new -std=c++17 -D__NuttX__ -pipe -nostdinc++
+ARCHCFLAGS = -fno-builtin -ffunction-sections -fdata-sections

Review comment:
       It isn't a big issue If ROM function doesn't use array argument, since the type will always promote to int per spec.




-- 
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] btashton commented on a change in pull request #5282: Update board Make.defs

Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #5282:
URL: https://github.com/apache/incubator-nuttx/pull/5282#discussion_r788898266



##########
File path: boards/risc-v/bl602/bl602evb/scripts/Make.defs
##########
@@ -45,8 +45,8 @@ ifneq ($(CONFIG_DEBUG_NOOPT),y)
 endif
 
 ARCHCPUFLAGS += -mno-relax
-ARCHCFLAGS = -fno-builtin -ffunction-sections -fdata-sections -fstrict-volatile-bitfields -fshort-enums -fno-omit-frame-pointer
-ARCHCXXFLAGS = -fno-builtin -ffunction-sections -fdata-sections -fstrict-volatile-bitfields -fshort-enums -fno-omit-frame-pointer -fno-exceptions -fcheck-new -std=c++17 -D__NuttX__ -pipe -nostdinc++
+ARCHCFLAGS = -fno-builtin -ffunction-sections -fdata-sections

Review comment:
       @xiaoxiang781216 if I recall this was required for abi compatibility when calling some ROM functions which unfortunately used enums as part of the API.  I know this is the case also with the Nordic Bluetooth softcore as well.  




-- 
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 #5282: Update board Make.defs

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5282:
URL: https://github.com/apache/incubator-nuttx/pull/5282#discussion_r788774671



##########
File path: boards/sparc/bm3803/xx3803/scripts/Make.defs
##########
@@ -55,7 +55,7 @@ ifeq ($(CONFIG_DEBUG_SYMBOLS),y)
 endif
 
 ifneq ($(CONFIG_DEBUG_NOOPT),y)
-  ARCHOPTIMIZATION += $(MAXOPTIMIZATION) -fno-strict-aliasing -fomit-frame-pointer
+  ARCHOPTIMIZATION += $(MAXOPTIMIZATION) -fno-strict-aliasing

Review comment:
       Don't need since I add to Toolchain.defs here:
   https://github.com/apache/incubator-nuttx/pull/5282/files#diff-956cdffe92103d6a46573f33066d5f0c0b4961f507a1defb5682c26cbeb544bbR82-R86

##########
File path: boards/sparc/bm3823/xx3823/scripts/Make.defs
##########
@@ -55,7 +55,7 @@ ifeq ($(CONFIG_DEBUG_SYMBOLS),y)
 endif
 
 ifneq ($(CONFIG_DEBUG_NOOPT),y)
-  ARCHOPTIMIZATION += $(MAXOPTIMIZATION) -fno-strict-aliasing -fomit-frame-pointer
+  ARCHOPTIMIZATION += $(MAXOPTIMIZATION) -fno-strict-aliasing

Review comment:
       ditto




-- 
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 #5282: Update board Make.defs

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5282:
URL: https://github.com/apache/incubator-nuttx/pull/5282#discussion_r788777548



##########
File path: boards/risc-v/bl602/bl602evb/scripts/Make.defs
##########
@@ -45,8 +45,8 @@ ifneq ($(CONFIG_DEBUG_NOOPT),y)
 endif
 
 ARCHCPUFLAGS += -mno-relax
-ARCHCFLAGS = -fno-builtin -ffunction-sections -fdata-sections -fstrict-volatile-bitfields -fshort-enums -fno-omit-frame-pointer
-ARCHCXXFLAGS = -fno-builtin -ffunction-sections -fdata-sections -fstrict-volatile-bitfields -fshort-enums -fno-omit-frame-pointer -fno-exceptions -fcheck-new -std=c++17 -D__NuttX__ -pipe -nostdinc++
+ARCHCFLAGS = -fno-builtin -ffunction-sections -fdata-sections

Review comment:
       > General question on `-fshort-enums`. Shouldn't this give better code size? 
   
   Since the prebuilt library(e.g. libgcc, libm, libsupc++) from toolchain never add -fshort-enums, it's wrong to enable this option unless we build the toolchain by self: https://oroboro.com/short-enum/
   
   > Do we have a Kconfig options to specify extra options like this? It might be needed if code is linked to third party pre-compiled libraries that are compiled with `-fshort-enums` and have pointers to enums or arrays of enums passed as parameters to functions.
   
   No. 
   




-- 
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 #5282: Update board Make.defs

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5282:
URL: https://github.com/apache/incubator-nuttx/pull/5282#discussion_r788785720



##########
File path: boards/xtensa/esp32/esp32-wrover-kit/scripts/Make.defs
##########
@@ -73,7 +73,7 @@ ifneq ($(CONFIG_DEBUG_NOOPT),y)
   ARCHOPTIMIZATION += $(MAXOPTIMIZATION) -fno-strict-aliasing
 endif
 
-ARCHCFLAGS = -fno-builtin -ffunction-sections -fdata-sections -fstrict-volatile-bitfields -mlongcalls
+ARCHCFLAGS = -fno-builtin -ffunction-sections -fdata-sections -mlongcalls

Review comment:
       ditto

##########
File path: boards/xtensa/esp32/esp32-devkitc/scripts/Make.defs
##########
@@ -73,7 +73,7 @@ ifneq ($(CONFIG_DEBUG_NOOPT),y)
   ARCHOPTIMIZATION += $(MAXOPTIMIZATION) -fno-strict-aliasing
 endif
 
-ARCHCFLAGS = -fno-builtin -ffunction-sections -fdata-sections -fstrict-volatile-bitfields -mlongcalls
+ARCHCFLAGS = -fno-builtin -ffunction-sections -fdata-sections -mlongcalls

Review comment:
       ditto




-- 
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 #5282: Update board Make.defs

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


   


-- 
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 #5282: Update board Make.defs

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5282:
URL: https://github.com/apache/incubator-nuttx/pull/5282#discussion_r788777548



##########
File path: boards/risc-v/bl602/bl602evb/scripts/Make.defs
##########
@@ -45,8 +45,8 @@ ifneq ($(CONFIG_DEBUG_NOOPT),y)
 endif
 
 ARCHCPUFLAGS += -mno-relax
-ARCHCFLAGS = -fno-builtin -ffunction-sections -fdata-sections -fstrict-volatile-bitfields -fshort-enums -fno-omit-frame-pointer
-ARCHCXXFLAGS = -fno-builtin -ffunction-sections -fdata-sections -fstrict-volatile-bitfields -fshort-enums -fno-omit-frame-pointer -fno-exceptions -fcheck-new -std=c++17 -D__NuttX__ -pipe -nostdinc++
+ARCHCFLAGS = -fno-builtin -ffunction-sections -fdata-sections

Review comment:
       > General question on `-fshort-enums`. Shouldn't this give better code size? 
   
   Since the prebuilt library(e.g. libgcc, libm, libsupc++) from toolchain never add -fshort-enums, it's wrong to enable this option unless we build the toolchain by self.
   
   > Do we have a Kconfig options to specify extra options like this? It might be needed if code is linked to third party pre-compiled libraries that are compiled with `-fshort-enums` and have pointers to enums or arrays of enums passed as parameters to functions.
   
   No. 
   




-- 
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 #5282: Update board Make.defs

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5282:
URL: https://github.com/apache/incubator-nuttx/pull/5282#discussion_r788775240



##########
File path: boards/xtensa/esp32s2/esp32s2-saola-1/scripts/Make.defs
##########
@@ -54,7 +54,7 @@ ifneq ($(CONFIG_DEBUG_NOOPT),y)
   ARCHOPTIMIZATION += $(MAXOPTIMIZATION) -fno-strict-aliasing
 endif
 
-ARCHCFLAGS = -fno-builtin -ffunction-sections -fdata-sections -fstrict-volatile-bitfields -mlongcalls
+ARCHCFLAGS = -fno-builtin -ffunction-sections -fdata-sections -mlongcalls

Review comment:
       it's strict-volatile-bitfields not -fomit-frame-pointer

##########
File path: boards/xtensa/esp32/ttgo_lora_esp32/scripts/Make.defs
##########
@@ -73,7 +73,7 @@ ifneq ($(CONFIG_DEBUG_NOOPT),y)
   ARCHOPTIMIZATION += $(MAXOPTIMIZATION) -fno-strict-aliasing
 endif
 
-ARCHCFLAGS = -fno-builtin -ffunction-sections -fdata-sections -fstrict-volatile-bitfields -mlongcalls
+ARCHCFLAGS = -fno-builtin -ffunction-sections -fdata-sections -mlongcalls

Review comment:
       ditto

##########
File path: boards/xtensa/esp32/esp32-ethernet-kit/scripts/Make.defs
##########
@@ -73,7 +73,7 @@ ifneq ($(CONFIG_DEBUG_NOOPT),y)
   ARCHOPTIMIZATION += $(MAXOPTIMIZATION) -fno-strict-aliasing
 endif
 
-ARCHCFLAGS = -fno-builtin -ffunction-sections -fdata-sections -fstrict-volatile-bitfields -mlongcalls
+ARCHCFLAGS = -fno-builtin -ffunction-sections -fdata-sections -mlongcalls

Review comment:
       ditto




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