Xiang, First let me say thank you for digging in to this.
The test case can relay influence the data. The table is a step function. One call +256, the others are subject to compiler magic. If we think along the lines of scalability, the table is fine on BIG un constrained systems. The calls are fine on MED systems and the macros are fine on SMALL (tiny) constrained systems. I think that vector is aligned with coding complexity and external lib usage as well. Would you agree, or are you using 3rd party lib's on tiny procs? This is why I have maintained the position it should be a Kconfig Knob. David -----Original Message----- From: Xiang Xiao [mailto:xiaoxiang781...@gmail.com] Sent: Friday, July 31, 2020 6:01 AM To: dev@nuttx.apache.org Subject: RE: [GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1487: libc: Avoid ctype function to evaluate the argument more than once Yes, you are right: function version is still 20Bytes bigger than macro version. Do you have a better solution to fix this problem? > -----Original Message----- > From: David Sidrane <david.sidr...@nscdg.com> > Sent: Friday, July 31, 2020 6:29 PM > To: dev@nuttx.apache.org > Subject: RE: [GitHub] [incubator-nuttx] xiaoxiang781216 commented on a > change in pull request #1487: libc: Avoid ctype function to > evaluate the argument more than once > > > The normal function size is smaller than macro version. > > The comment and data seem to be in conflict. > > master:43255 is less than 'normal function':43275 > > Am I missing something? > > -----Original Message----- > From: Xiang Xiao [mailto:xiaoxiang781...@gmail.com] > Sent: Friday, July 31, 2020 3:19 AM > To: dev@nuttx.apache.org > Subject: RE: [GitHub] [incubator-nuttx] xiaoxiang781216 commented on a > change in pull request #1487: libc: Avoid ctype function to > evaluate the argument more than once > > Here is the size report for stm32f103-minimum:nsh: > The master: > text data bss dec hex filename > 43255 108 2196 45559 b1f7 nuttx > With lookup table > patch(https://github.com/apache/incubator-nuttx/pull/1487): > text data bss dec hex filename > 43567 108 2196 45871 b32f nuttx > With normal function > patch(https://github.com/apache/incubator-nuttx/pull/1496): > text data bss dec hex filename > 43275 108 2196 45579 b20b nuttx > The normal function size is smaller than macro version. > > > -----Original Message----- > > From: Xiang Xiao <xiaoxiang781...@gmail.com> > > Sent: Friday, July 31, 2020 4:08 PM > > To: dev@nuttx.apache.org > > Subject: RE: [GitHub] [incubator-nuttx] xiaoxiang781216 commented on a > > change in pull request #1487: libc: Avoid ctype function to evaluate > > the argument more than once > > > > Let's restart the problem: it's definitely a bug that ctype macros > > evaluate it's argument more than once. If the code size is more > > important than the speed, let's change all macros to normal function > > that we get the correction behavior and save the code size at the same > > time. > > > > > -----Original Message----- > > > From: David Sidrane <david.sidr...@nscdg.com> > > > Sent: Friday, July 31, 2020 7:32 AM > > > To: dev@nuttx.apache.org > > > Subject: RE: [GitHub] [incubator-nuttx] xiaoxiang781216 commented on > > > a change in pull request #1487: libc: Avoid ctype function to > > > evaluate the argument more than once > > > > > > > Is this something we should be concerned about? > > > > > > > > > Sorry if that came off wrong. I was agreeing with Greg's with the > > > former statement. Not the latter one. > > > > > > David > > > > > > -----Original Message----- > > > From: Brennan Ashton [mailto:bash...@brennanashton.com] > > > Sent: Thursday, July 30, 2020 12:57 PM > > > To: dev@nuttx.apache.org > > > Subject: Re: [GitHub] [incubator-nuttx] xiaoxiang781216 commented on > > > a change in pull request #1487: libc: Avoid ctype function to > > > evaluate the argument more than once > > > > > > On Thu, Jul 30, 2020, 12:40 PM David Sidrane > > > <david.sidr...@nscdg.com> > > > wrote: > > > > > > > -1 on more bloat > > > > > > > > > > That is not a constructive vote. No one thinks we should have more > > > bloat. > > > > > > The question is do we need to focus more on this. Some of the > > > changes are due to shortcuts we have taken in the past that are now > > > causing issues. The PR that spurred this is exactly this. The code > > > as it exists is wrong in that it has unexpected side effects which > > > is what Xiao > > is trying to address. > > > > > > I will acknowledge that there have been other changes where it was > > > more of a complexity trade-off. Even on those there was discussion > > > about the impact. > > > > > > > > > > It is a simple matter to see the cost of a PR if we add bloaty > > > > (https://github.com/google/bloaty) to ci. > > > > > > > > > > Great please file the simple PR to add it. This is not trivial as we > > > need to manage the artifacts from previous builds to do the > > > comparison or double the build time. We heard you last time, but > > someone has to do to the actual work. > > > > > > --Brennan
