Re: Inlined code

2018-08-05 Thread Christian Mauderer
Am 05.08.2018 um 04:00 schrieb Chris Johns:
> Hi,
> 
> I have been working on migrating covoar in the rtems-tools repo to DWARF. The
> goal is remove objdump parsing and to get accurate details about the functions
> being covered. This is an unfunded task.
> 
> The work has resulted in a close examination of inlined code in RTEMS and 
> what I
> saw alarmed me so I have added a report to the rtems-exeinfo tool in 
> rtems-tools
> (the change is to be posted for review once I get the coverage tests running).
> 
> A summary report for hello.exe on RTEMS 5 for SPARC is:
> 
> inlined funcs   : 1412
> total funcs : 1956
>  % inline funcs : 72%
>  total size : 174616
> inline size : 81668
>   % inline size : 46%
> 
> This is a small application so it could be argued that skews the figures. A
> large C/C++ application built with -O2 running on RTEMS 4.11 ARM reports the
> inline usage as:
> 
> inlined funcs   : 10370
> total funcs : 17700
>  % inline funcs : 58%
>  total size : 3066240
> inline size : 1249514
>   % inline size : 40%
> 
> This does not seem right to me.
> 
> The report is new and there could be issues in the DWARF handling that feeds
> this report however I am posting this to start a discussion on the topic of
> inlining.
> 
> I attach the report for hello.exe. The `-i` option generates the inline 
> report.
> 
> The first section is a summary showing the total number of functions in the
> executable that have machine code and are flagged as inline. The report lists
> the percentage of functions that are inlined and the percentage of machine 
> code
> that is inlined. The values seem high to me.
> 
> The second table lists inline functions that are repeated sorted from the
> largest foot print to the smallest. The first column the total size of machine
> code in the executable and the second column the number of instances.
> 
> The third table is the list of inline functions sorted from largest machine 
> code
> footprint to smallest. The second column are flags of which there is one. A 
> `E`
> indicates the inline function is also external which means the compiler has
> created an external reference to this function, ie an address-of is being 
> taken.
> The third column is the address in the executable so you can take a look with
> objdump at the machine code.
> 
> We need to ask some important question in relation to inlining. It is cheap to
> add and we all feel the code we add needs to be fast and needs to be inlined 
> but
> does it really need to be inlined?
> 
> Some pieces of code do need to be inlined and the overhead is just that an
> overhead, for example in the large C/C++ application there is a low level
> volatile hardware write routing with close to 300 instances and a code size of
> 10K. This code needs to be inlined for performance reasons but should the size
> on average be 40 bytes, I doubt it.
> 
> Can we be more judicious with our use of the inline keyword?
> 
> Is the performance gain we really expect or is the actual overhead of a call
> frame not worth saving?
> 
> What are the real costs of inlining a piece of code? It adds size to the
> executable and depending on the code being inlined it complicates coverage
> analysis be adding extra branch points.
> 
> The metrics to determine what should be inlined is complicated and I do not
> think we have a suitable policy in place. I believe it is time we to create 
> one.
> 
> The issue is not limited to our code, gcc, newlib and libstdc++ seem to have
> some code that should be looked at more closely. For example __udivmoddi4, and
> __sprint_r.
> 
> Chris
> 
> 

Hello Chris,

I just took a look at one of the first function in your list: __sprint_r

https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;a=blob;f=newlib/libc/stdio/vfprintf.c;h=c4bf2dbe31da64462ecccec97c8e901e4ffadd44;hb=HEAD#l403

As far as I can see, there is no explicit inline key word for that
function. So in that case, the compiler decided that it would be a good
idea to inline that function.

I'm not sure whether I might just haven't seen it but is there a
possibility to distinguish between functions that have been inlined by
the compiler and ones that have been inlined due to the "inline" keyword
without looking at every definition?

Did you try compiling with size optimization? I would expect that the
compiler would inline far less functions and maybe even ignore some
"inline" keywords. As far as I know it's more of a hint to the compiler.

I would only worry about functions that are still inlined if size
optimization is selected. That's the case when I tell the compiler to
make the program as small as possible. In all other cases I want some
well balanced optimum between speed and size. Inlining small functions
is OK in that case if you ask me.

Best regards

Christian
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


Re: Version of FreeBSD for libbsd

2018-08-05 Thread Christian Mauderer
Am 03.08.2018 um 19:30 schrieb Joel Sherrill:
> Hi
> 
> libbsd.txt says this is based on FreeBSD 9.2 but I thought it
> had been updated past that.
> 
> Also how could I have figured this out otherwise?
> 
> If it matters, there are also man pages references to FreeBSD 9.2.
> 
> And is the Changes up to date? It has a date of 2017?
> 
> Thanks.
> 
> 
> --joel
> 
> 

Hello Joel,

since we have a single release for all sources since about one year, the
simplest method to find out which release is used is to take a look at
the freebsd-org submodule. Currently that's revision 642b174da of the
FreeBSD git mirror which is from 2017-04-04.

So it should be something between 11 and 12.

Best regards

Christian
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


Re: Version of FreeBSD for libbsd

2018-08-05 Thread Joel Sherrill
On Sun, Aug 5, 2018, 8:59 AM Christian Mauderer  wrote:

> Am 03.08.2018 um 19:30 schrieb Joel Sherrill:
> > Hi
> >
> > libbsd.txt says this is based on FreeBSD 9.2 but I thought it
> > had been updated past that.
> >
> > Also how could I have figured this out otherwise?
> >
> > If it matters, there are also man pages references to FreeBSD 9.2.
> >
> > And is the Changes up to date? It has a date of 2017?
> >
> > Thanks.
> >
> >
> > --joel
> >
> >
>
> Hello Joel,
>
> since we have a single release for all sources since about one year, the
> simplest method to find out which release is used is to take a look at
> the freebsd-org submodule. Currently that's revision 642b174da of the
> FreeBSD git mirror which is from 2017-04-04.
>
> So it should be something between 11 and 12.
>

Then what do you recommend the phrase be in the libbsd.txt since it says 9
right now.

And should the man page links be updated? I know they rarely change but...

Thanks.

>
> Best regards
>
> Christian
>
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: Version of FreeBSD for libbsd

2018-08-05 Thread Christian Mauderer
Am 05.08.2018 um 19:21 schrieb Joel Sherrill:
> 
> 
> On Sun, Aug 5, 2018, 8:59 AM Christian Mauderer  > wrote:
> 
> Am 03.08.2018 um 19:30 schrieb Joel Sherrill:
> > Hi
> >
> > libbsd.txt says this is based on FreeBSD 9.2 but I thought it
> > had been updated past that.
> >
> > Also how could I have figured this out otherwise?
> >
> > If it matters, there are also man pages references to FreeBSD 9.2.
> >
> > And is the Changes up to date? It has a date of 2017?
> >
> > Thanks.
> >
> >
> > --joel
> >
> >
> 
> Hello Joel,
> 
> since we have a single release for all sources since about one year, the
> simplest method to find out which release is used is to take a look at
> the freebsd-org submodule. Currently that's revision 642b174da of the
> FreeBSD git mirror which is from 2017-04-04.
> 
> So it should be something between 11 and 12.
> 
> 
> Then what do you recommend the phrase be in the libbsd.txt since it says
> 9 right now.

Currently it's

  RTEMS uses FreeBSD 9.2 as the source of its TCP/IP and USB stacks.

How about something like the following instead:

  The libbsd makes FreeBSD subsystems like TCP/IP, USB, SD and some more
usable for RTEMS. It tries to follow the FreeBSD development as close as
possible and therefore is updated to the latest FreeBSD HEAD revision
from time to time.

It seems that we have two types of man page links:

1. In the "Network Stack Features" section and two other points:
http://www.freebsd.org/cgi/man.cgi?query=unix&sektion=4&apropos=0&manpath=FreeBSD+9.2-RELEASE[UNIX(4)]::
UNIX-domain protocol family

2. The following style for all other links:
http://www.freebsd.org/cgi/man.cgi?query=device

I'm not sure whether the "Network Stack Features" is still up to date
but if it is, maybe we should migrate to the style 2.

Best regards

Christian

> 
> And should the man page links be updated? I know they rarely change but...
> 
> Thanks.
> 
> 
> Best regards
> 
> Christian
> 

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


Re: [PATCH v2] Coverage: Add support to generate separate reports for each symbol-set

2018-08-05 Thread Vijay Kumar Banerjee
ping :)

On 3 August 2018 at 03:15, Vijay Kumar Banerjee 
wrote:

> Hello,
>
> If you find some time, please do a final review of this patch and provide
> suggestions if it's not mergeable yet :)
>
> There are only three days left, I seek some advice on the wrapup work on
> coverage
> analysis . The current status is :
>
> * This patch adds support to generate separate report for each symbol-sets.
>
> * I sent a patch a day ago with the symbol-sets and the respective
> libraray
>   addresses added to the symbol-sets.ini file. I have also attached a
> screenshot of the report.
>   Joel, does the report look good to you ?
>   What are the next steps to get the reports published ? Is it intended to
> be a
>   post GSoC work ?
>
> * I have completed the dumper for gcno files. There are still more
> understanding
>   needed to make sense out of the data, but all the data can be dumped in
> a human
>   readable format now. Please have a look at the  generated txt file.
>   https://github.com/thelunatic/gcno_dumper/blob/master/gcno_dump.txt
> 
>
> *  Any other suggestions ?
>
> Thanks
> --vijayk
>
> On 30 July 2018 at 22:55, Vijay Kumar Banerjee 
> wrote:
>
>> Invoke covoar multiple times from the script to generate separate
>> reports for each symbol-set.
>> ---
>>  tester/rt/coverage.py | 38 ++
>>  1 file changed, 18 insertions(+), 20 deletions(-)
>>
>> diff --git a/tester/rt/coverage.py b/tester/rt/coverage.py
>> index 7dd5002..c979332 100644
>> --- a/tester/rt/coverage.py
>> +++ b/tester/rt/coverage.py
>> @@ -99,8 +99,8 @@ class summary:
>>  return line.strip().split(' ')[0]
>>
>>  class report_gen_html:
>> -def __init__(self, p_symbol_sets_list, build_dir, rtdir, bsp):
>> -self.symbol_sets_list = ['score']
>> +def __init__(self, symbol_sets, build_dir, rtdir, bsp):
>> +self.symbol_sets = symbol_sets
>>  self.build_dir = build_dir
>>  self.partial_reports_files = list(['index.html', 'summary.txt'])
>>  self.number_of_columns = 1
>> @@ -109,7 +109,7 @@ class report_gen_html:
>>
>>  def _find_partial_reports(self):
>>  partial_reports = {}
>> -for symbol_set in self.symbol_sets_list:
>> +for symbol_set in self.symbol_sets:
>>  set_summary = summary(path.join(self.bsp + "-coverage",
>>  symbol_set))
>>  set_summary.parse()
>> @@ -204,7 +204,7 @@ class report_gen_html:
>>  def add_covoar_src_path(self):
>>  table_js_path = path.join(self.covoar_src_path, 'table.js')
>>  covoar_css_path = path.join(self.covoar_src_path, 'covoar.css')
>> -for symbol_set in self.symbol_sets_list:
>> +for symbol_set in self.symbol_sets:
>>  symbol_set_dir = path.join(self.build_dir,
>> self.bsp + '-coverage',
>> symbol_set)
>>  html_files = os.listdir(symbol_set_dir)
>> @@ -264,27 +264,23 @@ class symbol_parser(object):
>>  for sset in self.ssets:
>>  lib = path.join(self.build_dir, config.get('libraries',
>> sset))
>>  self.symbol_sets[sset] = lib.encode('utf-8')
>> +return self.ssets
>>  except:
>>  raise error.general('Symbol set parsing failed')
>>
>> -def _write_ini(self):
>> +def write_ini(self, symbol_set):
>>  config = configparser.ConfigParser()
>>  try:
>> -sets = ', '.join(self.symbol_sets.keys())
>> +sset = symbol_set
>>  config.add_section('symbol-sets')
>> -config.set('symbol-sets', 'sets', sets)
>> -for key in self.symbol_sets.keys():
>> -config.add_section(key)
>> -config.set(key, 'libraries', self.symbol_sets[key])
>> +config.set('symbol-sets', 'sets', sset)
>> +config.add_section(sset)
>> +config.set(sset, 'libraries', self.symbol_sets[sset])
>>  with open(self.symbol_select_file, 'w') as conf:
>>  config.write(conf)
>>  except:
>>  raise error.general('symbol parser write failed')
>>
>> -def run(self):
>> -self.parse()
>> -self._write_ini()
>> -
>>  class covoar(object):
>>  '''
>>  Covoar runner
>> @@ -371,20 +367,22 @@ class coverage_run(object):
>> self.symbol_select_path,
>> self.symbol_set,
>> build_dir)
>> -parser.run()
>> -covoar_runner = covoar(self.test_dir,
>> self.symbol_select_path,
>> +symbol_sets = parser.parse()
>> +for sset in symbol_sets:
>> +parser.write_ini(sset)
>> +covoar_runner = covoar(self.test_dir,
>> self.symbol_select_path,
>>  

Re: Version of FreeBSD for libbsd

2018-08-05 Thread Joel Sherrill
On Sun, Aug 5, 2018, 3:25 PM Christian Mauderer  wrote:

> Am 05.08.2018 um 19:21 schrieb Joel Sherrill:
> >
> >
> > On Sun, Aug 5, 2018, 8:59 AM Christian Mauderer  > > wrote:
> >
> > Am 03.08.2018 um 19:30 schrieb Joel Sherrill:
> > > Hi
> > >
> > > libbsd.txt says this is based on FreeBSD 9.2 but I thought it
> > > had been updated past that.
> > >
> > > Also how could I have figured this out otherwise?
> > >
> > > If it matters, there are also man pages references to FreeBSD 9.2.
> > >
> > > And is the Changes up to date? It has a date of 2017?
> > >
> > > Thanks.
> > >
> > >
> > > --joel
> > >
> > >
> >
> > Hello Joel,
> >
> > since we have a single release for all sources since about one year,
> the
> > simplest method to find out which release is used is to take a look
> at
> > the freebsd-org submodule. Currently that's revision 642b174da of the
> > FreeBSD git mirror which is from 2017-04-04.
> >
> > So it should be something between 11 and 12.
> >
> >
> > Then what do you recommend the phrase be in the libbsd.txt since it says
> > 9 right now.
>
> Currently it's
>
>   RTEMS uses FreeBSD 9.2 as the source of its TCP/IP and USB stacks.
>
> How about something like the following instead:
>
>   The libbsd makes FreeBSD subsystems like TCP/IP, USB, SD and some more
> usable for RTEMS. It tries to follow the FreeBSD development as close as
> possible and therefore is updated to the latest FreeBSD HEAD revision
> from time to time.
>

This is much better. Before it was dated and I knew is was wrong.bu5 not
how to fix it.

Could it also provide a pointer/link to info in the FreeBSD submodule so
someone can pull it and answer the question precisely?

I was looking for a specific answer to put in the paper we have been
working on. It shouldn't be hard or require too much magic to answer this
question. :)

>
> It seems that we have two types of man page links:
>
> 1. In the "Network Stack Features" section and two other points:
>
> http://www.freebsd.org/cgi/man.cgi?query=unix&sektion=4&apropos=0&manpath=FreeBSD+9.2-RELEASE[UNIX(4)]
> ::
> UNIX-domain protocol family
>
> 2. The following style for all other links:
> http://www.freebsd.org/cgi/man.cgi?query=device
>
> I'm not sure whether the "Network Stack Features" is still up to date
> but if it is, maybe we should migrate to the style 2.
>

As long as we are tracking reasonably close to their master, would it not
make sense to just use their master docs? Which I think is the latter form.

Thanks.

--joel

>
> Best regards
>
> Christian
>
> >
> > And should the man page links be updated? I know they rarely change
> but...
> >
> > Thanks.
> >
> >
> > Best regards
> >
> > Christian
> >
>
>
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

[PATCH 1/5] rtemstoolkit: Various coverity related fixes.

2018-08-05 Thread Chris Johns
---
 rtemstoolkit/rld-elf.cpp |  1 +
 rtemstoolkit/rld-process.cpp | 21 ++---
 rtemstoolkit/rld-rtems.cpp   |  1 -
 3 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/rtemstoolkit/rld-elf.cpp b/rtemstoolkit/rld-elf.cpp
index ace3967..231f2bf 100644
--- a/rtemstoolkit/rld-elf.cpp
+++ b/rtemstoolkit/rld-elf.cpp
@@ -425,6 +425,7 @@ namespace rld
 archive (false),
 writable (false),
 elf_ (0),
+mtype (0),
 oclass (0),
 ident_str (0),
 ident_size (0),
diff --git a/rtemstoolkit/rld-process.cpp b/rtemstoolkit/rld-process.cpp
index e91796f..30e0605 100644
--- a/rtemstoolkit/rld-process.cpp
+++ b/rtemstoolkit/rld-process.cpp
@@ -83,7 +83,13 @@ namespace rld
 
 temporary_files::~temporary_files ()
 {
-  clean_up ();
+  try
+  {
+clean_up ();
+  }
+  catch (...)
+  {
+  }
 }
 
 const std::string
@@ -99,6 +105,9 @@ namespace rld
   RLD_PATH_SEPARATOR_STR);
   tempfile_ref ref (name, keep);
   tempfiles.push_back (ref);
+
+  ::free (temp);
+
   return name;
 }
 
@@ -162,8 +171,14 @@ namespace rld
 
 tempfile::~tempfile ()
 {
-  close ();
-  temporaries.erase (_name);
+  try
+  {
+close ();
+temporaries.erase (_name);
+  }
+  catch (...)
+  {
+  }
 }
 
 void
diff --git a/rtemstoolkit/rld-rtems.cpp b/rtemstoolkit/rld-rtems.cpp
index 806a2e1..a2dcf82 100644
--- a/rtemstoolkit/rld-rtems.cpp
+++ b/rtemstoolkit/rld-rtems.cpp
@@ -210,7 +210,6 @@ namespace rld
   if (slash == std::string::npos)
 throw rld::error ("Invalid BSP name", _arch_bsp);
   return _arch_bsp.substr (0, slash);
-  std::string bsp  = _arch_bsp.substr (slash + 1);
 }
 
 const std::string
-- 
2.15.1

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


[PATCH] DWARF and Coverage Integration

2018-08-05 Thread Chris Johns
Hello,

This patch series includes all my changes so covoar can use DWARF derived
function data when creating the coverage maps.

Using DWARF data means there are now non-continous ranges and CoverageMapBase
has been updated to cleanly handle ranges.

DWARF data also gives us more detail about the functions which adds some
complexity. Inline functions can be external and objdump seems to show an
instance of such a function as if it is a normal or concrete function. As a
result the objdump parser expects a coverage map for those symbols to exist.
The side effect of adding these external inline function is the ability for
them to vary in size in different executables and they do.

These patches include the inline and DWARF reports.

Chris

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


[PATCH 3/5] rtemstoolkit/dwarf: C++ object relates fixes and a dump report.

2018-08-05 Thread Chris Johns
- Various C++ object fixes that improve stability where data from
  libdwarf is moving between object instances.
- Functions now provide better detail with inlined functions picking
  up attributes from an abstrtact DIE.
- Dump to a provide stream not stdout.
---
 rtemstoolkit/rld-dwarf.cpp | 504 +
 rtemstoolkit/rld-dwarf.h   | 122 +--
 2 files changed, 477 insertions(+), 149 deletions(-)

diff --git a/rtemstoolkit/rld-dwarf.cpp b/rtemstoolkit/rld-dwarf.cpp
index 9b92ec0..9cdd84a 100644
--- a/rtemstoolkit/rld-dwarf.cpp
+++ b/rtemstoolkit/rld-dwarf.cpp
@@ -82,7 +82,7 @@ namespace rld
 std::ostringstream exe_where;
 std::stringwhat;
 exe_where << "dwarf:" << where;
-what = dwarf_errmsg (error);
+what = ::dwarf_errmsg (error);
 throw rld::error (what, exe_where.str ());
   }
 }
@@ -299,7 +299,7 @@ namespace rld
 }
 
 void
-range::dump ()
+range::dump (std::ostream& out) const
 {
   dwarf_ranges_type type_ = type ();
   const char*   type_s = "invalid";
@@ -310,10 +310,11 @@ namespace rld
   };
   if (type_ <= DW_RANGES_END)
 type_s = type_labels[type_];
-  std::cout << type_s << '-'
-<< std::hex << std::setfill ('0')
-<< "0x" << std::setw (8) << addr1 ()
-<< ":0x" << std::setw (8) << addr2 ();
+  out << type_s << '-'
+  << std::hex << std::setfill ('0')
+  << "0x" << std::setw (8) << addr1 ()
+  << ":0x" << std::setw (8) << addr2 ()
+  << std::dec << std::setfill (' ');
 }
 
 address_ranges::address_ranges (file& debug)
@@ -344,7 +345,9 @@ namespace rld
 
 address_ranges::address_ranges (const address_ranges& orig)
   : debug (orig.debug),
-offset (orig.offset)
+offset (orig.offset),
+dranges (nullptr),
+dranges_count (0)
 {
   load (orig.offset);
 }
@@ -392,6 +395,8 @@ namespace rld
 if (dranges != nullptr)
   ::dwarf_ranges_dealloc (debug, dranges, dranges_count);
 
+ranges_.clear ();
+
 dranges = nullptr;
 dranges_count = 0;
 
@@ -444,18 +449,18 @@ namespace rld
 }
 
 void
-address_ranges::dump ()
+address_ranges::dump (std::ostream& out) const
 {
   bool first = true;
-  std::cout << '[';
+  out << '[';
   for (auto& r : ranges_)
   {
 if (!first)
-  std::cout << ',';
-r.dump ();
+  out << ',';
+r.dump (out);
 first = false;
   }
-  std::cout << ']';
+  out << ']';
 }
 
 line_addresses::line_addresses (file& debug,
@@ -576,9 +581,12 @@ namespace rld
 external_ (false),
 declaration_ (false),
 inline_ (DW_INL_not_inlined),
+entry_pc_ (0),
+has_entry_pc_ (false),
 pc_low_ (0),
 pc_high_ (0),
-ranges_ (debug)
+ranges_ (debug),
+call_line_ (0)
 {
   dwarf_bool db;
 
@@ -588,16 +596,18 @@ namespace rld
   if (die.attribute (DW_AT_declaration, db, false))
 declaration_ = db ? true : false;
 
+  if (die.attribute (DW_AT_entry_pc, entry_pc_, false))
+has_entry_pc_ = true;
+
   die.attribute (DW_AT_linkage_name, linkage_name_, false);
+  die.attribute (DW_AT_call_file, decl_file_, false);
+  die.attribute (DW_AT_call_line, decl_line_, false);
+  die.attribute (DW_AT_call_file, call_file_, false);
+  die.attribute (DW_AT_call_line, call_line_, false);
 
   if (!die.attribute (DW_AT_inline, inline_, false))
 inline_ = DW_INL_not_inlined;
 
-  if (inline_ == DW_INL_declared_inlined)
-  {
-die_dump_children (die, " +");
-  }
-
   /*
* If ranges are not found see if the PC low and PC high attributes
* can be found.
@@ -653,7 +663,17 @@ namespace rld
 {
   debug_info_entry abst_at_die (debug, abst_at_die_offset);
   if (abst_at_die.attribute (DW_AT_name, name_, false))
+  {
 found = true;
+abst_at_die.attribute (DW_AT_inline, inline_, false);
+if (abst_at_die.attribute (DW_AT_external, db, false))
+  external_ = db ? true : false;
+if (abst_at_die.attribute (DW_AT_declaration, db, false))
+  declaration_ = db ? true : false;
+abst_at_die.attribute (DW_AT_linkage_name, linkage_name_, 
false);
+abst_at_die.attribute (DW_AT_decl_file, decl_file_, false);
+abst_at_die.attribute (DW_AT_decl_line, decl_line_, false);
+  }
 }
   }
 
@@ -675,36 +695,48 @@ namespace rld
   {
 debug_info_entry spec_die (debug, spec_die_offset);
 if (spec_die.attribute (DW_AT_name, name_, false))
+{
   found =

[PATCH 5/5] tester/covoar: Integrate DWARF function data.

2018-08-05 Thread Chris Johns
Use DAWRF function data to create the executable coverage
maps. Integrate the existing objdump processing with this
data.

- Refactor CoverageMapBase to have the address ranges
  and address info as separate objects. Move the
  to address info into a vector. Add support for
  multiple address ranges.
- DesiredSymbols is only interested in function symbols.
- ExecutableInfo creates coverage maps from DWARF function
  data.
- Add warning flags to the covoar build.
- Varous C++11 refactoring.
---
 tester/covoar/CoverageMapBase.cc  | 372 ++
 tester/covoar/CoverageMapBase.h   | 185 ++-
 tester/covoar/DesiredSymbols.cc   |  33 ++--
 tester/covoar/ExecutableInfo.cc   |  41 -
 tester/covoar/ExecutableInfo.h|  32 ++--
 tester/covoar/ObjdumpProcessor.cc | 122 +++--
 tester/covoar/covoar.cc   |  20 +-
 tester/covoar/wscript |   4 +-
 8 files changed, 381 insertions(+), 428 deletions(-)

diff --git a/tester/covoar/CoverageMapBase.cc b/tester/covoar/CoverageMapBase.cc
index 87c8e8f..ad0080d 100644
--- a/tester/covoar/CoverageMapBase.cc
+++ b/tester/covoar/CoverageMapBase.cc
@@ -11,98 +11,135 @@
 #include 
 #include 
 
+#include 
+
 #include "CoverageMapBase.h"
 
 namespace Coverage {
 
-  CoverageMapBase::CoverageMapBase(
-const std::string& exefileName,
-uint32_t   low,
-uint32_t   high
-  )
+  AddressInfo::AddressInfo ()
+: isStartOfInstruction (false),
+  wasExecuted (false),
+  isBranch (false),
+  isNop (false),
+  wasTaken (false),
+  wasNotTaken (false)
   {
-uint32_t a;
-AddressRange range;
+  }
 
-range.fileName= exefileName;
-range.lowAddress  = low;
-range.highAddress = high;
-Ranges.push_back( range );
+  AddressRange::AddressRange ()
+: lowAddress(0),
+  highAddress(0)
+  {
+  }
 
-Size = high - low + 1;
+  AddressRange::AddressRange (const std::string& name,
+  uint32_t   lowAddress,
+  uint32_t   highAddress)
+: fileName (name),
+  lowAddress (lowAddress),
+  highAddress (highAddress)
+  {
+info.resize( size( ) );
+  }
 
-Info = new perAddressInfo[ Size ];
+  size_t AddressRange::size () const
+  {
+return highAddress - lowAddress + 1;
+  }
 
-for (a = 0; a < Size; a++) {
+  bool AddressRange::inside (uint32_t address) const
+  {
+return address >= lowAddress && address <= highAddress;
+  }
 
-  perAddressInfo *i = &Info[ a ];
+  AddressInfo& AddressRange::get (uint32_t address)
+  {
+if ( !inside( address ) )
+  throw rld::error( "address outside range", "AddressRange::get" );
+size_t slot = address - lowAddress;
+if (slot >= info.size ())
+  throw rld::error( "address slot not found", "AddressRange::get" );
+return info[slot];
+  }
 
-  i->isStartOfInstruction = false;
-  i->wasExecuted  = 0;
-  i->isBranch = false;
-  i->isNop= false;
-  i->wasTaken = 0;
-  i->wasNotTaken  = 0;
-}
+  const AddressInfo& AddressRange::get (uint32_t address) const
+  {
+if ( !inside( address ) )
+  throw rld::error( "address outside range", "AddressRange::get" );
+size_t slot = address - lowAddress;
+if (slot >= info.size ())
+  throw rld::error( "address slot not found", "AddressRange::get" );
+return info[slot];
   }
 
-  CoverageMapBase::~CoverageMapBase()
+  void AddressRange::dump (std::ostream& out, bool show_slots) const
   {
-if (Info)
-  delete Info;
+out << std::hex << std::setfill('0')
+<< "Address range: low = " << std::setw(8) << lowAddress
+<< " high = " << std::setw(8) << highAddress
+<< std::endl;
+if (show_slots) {
+  size_t slot = 0;
+  for (auto& i : info) {
+out << std::hex << std::setfill('0')
+<< "0x" << std::setw(8) << slot++ + lowAddress
+<< "- isStartOfInstruction:"
+<< (char*) (i.isStartOfInstruction ? "yes" : "no")
+<< " wasExecuted:"
+<< (char*) (i.wasExecuted ? "yes" : "no")
+<< std::endl
+<< "   isBranch:"
+<< (char*) (i.isBranch ? "yes" : "no")
+<< " wasTaken:"
+<< (char*) (i.wasTaken ? "yes" : "no")
+<< " wasNotTaken:"
+<< (char*) (i.wasNotTaken ? "yes" : "no")
+<< std::dec << std::setfill(' ')
+<< std::endl;
+  }
+}
   }
 
-  void  CoverageMapBase::Add( uint32_t low, uint32_t high )
+  CoverageMapBase::CoverageMapBase(
+const std::string& exefileName,
+uint32_t   low,
+uint32_t   high
+) : exefileName (exefileName)
   {
-AddressRange range;
+Ranges.push_back( AddressRange( exefileName, low, high ) );
+  }
 
-range.lowAddress  = low;
-range.highAddress = high;
-Ranges.push_back( range );

[PATCH 2/5] rtemstoolkit/elf-symbols: Add the symbol types as an enum.

2018-08-05 Thread Chris Johns
This can be used by applications to filter the symbols by type.
---
 rtemstoolkit/rld-symbols.h | 21 +
 1 file changed, 21 insertions(+)

diff --git a/rtemstoolkit/rld-symbols.h b/rtemstoolkit/rld-symbols.h
index 28d3cab..c3035c4 100644
--- a/rtemstoolkit/rld-symbols.h
+++ b/rtemstoolkit/rld-symbols.h
@@ -61,6 +61,27 @@ namespace rld
 class symbol
 {
 public:
+  /**
+   * Types of symbols.
+   */
+  enum type {
+st_notype = STT_NOTYPE,/* unspecified type */
+st_object = STT_OBJECT,/* data object */
+st_func = STT_FUNC,/* executable code */
+st_section = STT_SECTION,  /* section */
+st_file = STT_FILE,/* source file */
+st_common = STT_COMMON,/* uninitialized common block */
+st_tls = STT_TLS,  /* thread local storage */
+st_loos = STT_LOOS,/* start of OS-specific types */
+st_gnu_ifunc = STT_GNU_IFUNC,  /* indirect function */
+st_hios = STT_HIOS,/* end of OS-specific types */
+st_loproc = STT_LOPROC,/* start of processor-specific 
types */
+st_arm_tfunc = STT_ARM_TFUNC,  /* Thumb function (GNU) */
+st_arm_16bit = STT_ARM_16BIT,  /* Thumb label (GNU) */
+st_sparc_reg = STT_SPARC_REGISTER, /* SPARC register information */
+st_hiproc = STT_HIPROC,/* end of processor-specific types 
*/
+  };
+
   /**
* Default constructor. No symbol has been defined.
*/
-- 
2.15.1

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


[PATCH 4/5] linkers/exeinfo: Add an inlines report and DWARF data dump.

2018-08-05 Thread Chris Johns
---
 linkers/rtems-exeinfo.cpp | 133 --
 1 file changed, 130 insertions(+), 3 deletions(-)

diff --git a/linkers/rtems-exeinfo.cpp b/linkers/rtems-exeinfo.cpp
index 4fc5f00..f42cba1 100644
--- a/linkers/rtems-exeinfo.cpp
+++ b/linkers/rtems-exeinfo.cpp
@@ -174,6 +174,16 @@ namespace rld
* Output init/fini worker.
*/
   void output_init_fini (const char* label, const char** names);
+
+  /*
+   * Output the inlined functions.
+   */
+  void output_inlined ();
+
+  /*
+   * Output the DWARF data.
+   */
+  void output_dwarf ();
 };
 
 section::section (const files::section& sec, files::byteorder byteorder)
@@ -294,6 +304,7 @@ namespace rld
   exe.load_symbols (symbols, true);
   debug.load_debug ();
   debug.load_types ();
+  debug.load_functions ();
   symbols.globals (addresses);
   symbols.weaks (addresses);
   symbols.locals (addresses);
@@ -600,6 +611,103 @@ namespace rld
 
   std::cout << std::endl;
 }
+
+struct func_count
+{
+  std::string name;
+  int count;
+  size_t  size;
+
+  func_count (std::string name, size_t size)
+: name (name),
+  count (1),
+  size (size) {
+  }
+};
+typedef std::vector < func_count > func_counts;
+
+void image::output_inlined ()
+{
+  size_t   total = 0;
+  size_t   total_size = 0;
+  size_t   inlined_size = 0;
+  dwarf::functions funcs;
+  func_counts  counts;
+
+  for (auto& cu : debug.get_cus ())
+  {
+for (auto& f : cu.get_functions ())
+{
+  if (f.size () > 0 && f.has_machine_code ())
+  {
+++total;
+total_size += f.size ();
+if (f.is_inlined ())
+{
+  inlined_size += f.size ();
+  bool counted = false;
+  for (auto& c : counts)
+  {
+if (c.name == f.name ())
+{
+  ++c.count;
+  c.size += f.size ();
+  counted = true;
+  break;
+}
+  }
+  if (!counted)
+counts.push_back (func_count (f.name (), f.size ()));
+  funcs.push_back (f);
+}
+  }
+}
+  }
+
+  std::cout << "inlined funcs   : " << funcs.size () << std::endl
+<< "total funcs : " << total << std::endl
+<< " % inline funcs : " << (funcs.size () * 100) / total << '%'
+<< std::endl
+<< " total size : " << total_size << std::endl
+<< "inline size : " << inlined_size << std::endl
+<< "  % inline size : " << (inlined_size * 100) / total_size 
<< '%'
+<< std::endl;
+
+  auto count_compare = [](func_count const & a, func_count const & b) {
+return a.size != b.size?  a.size < b.size : a.count > b.count;
+  };
+  std::sort (counts.begin (), counts.end (), count_compare);
+  std::reverse (counts.begin (), counts.end ());
+
+  std::cout  << std::endl << "inlined repeats : " << std::endl;
+  for (auto& c : counts)
+if (c.count > 1)
+  std::cout << std::setw (6) << c.size << ' '
+<< std::setw (4) << c.count << ' '
+<< c.name << std::endl;
+
+  std::cout << std::endl << "inline funcs : " << std::endl;
+  dwarf::function_compare compare (dwarf::function_compare::fc_by_size);
+  std::sort (funcs.begin (), funcs.end (), compare);
+  std::reverse (funcs.begin (), funcs.end ());
+
+  for (auto& f : funcs)
+  {
+std::cout << std::setw (6) << f.size () << ' '
+  << (char) (f.is_external () ? 'E' : ' ')
+  << std::hex << std::setfill ('0')
+  << " 0x" << std::setw (8) << f.pc_low ()
+  << std::dec << std::setfill (' ')
+  << ' ' << f.name ()
+  << std::endl;
+  }
+}
+
+void image::output_dwarf ()
+{
+  std::cout << "DWARF Data:" << std::endl;
+  debug.dump (std::cout);
+}
   }
 }
 
@@ -616,6 +724,8 @@ static struct option rld_opts[] = {
   { "sections",no_argument,NULL,   'S' },
   { "init",no_argument,NULL,   'I' },
   { "fini",no_argument,NULL,   'F' },
+  { "inlined", no_argument,NULL,   'i' },
+  { "dwarf",   no_argument,NULL,   'D' },
   { NULL,  0,  NULL,0 }
 };
 
@@ -629,11 +739,13 @@ usage (int exit_code)
 << " -v: verbose (trace import parts), can supply multiple 
times" << std::endl
 << " to increase verbosity (also --verbose)" << 
std::endl
 << " -M

Re: Inlined code

2018-08-05 Thread Chris Johns
On 05/08/2018 19:39, Christian Mauderer wrote:
> Am 05.08.2018 um 04:00 schrieb Chris Johns:
>> Hi,
>>
>> I have been working on migrating covoar in the rtems-tools repo to DWARF. The
>> goal is remove objdump parsing and to get accurate details about the 
>> functions
>> being covered. This is an unfunded task.
>>
>> The work has resulted in a close examination of inlined code in RTEMS and 
>> what I
>> saw alarmed me so I have added a report to the rtems-exeinfo tool in 
>> rtems-tools
>> (the change is to be posted for review once I get the coverage tests 
>> running).
>>
>> A summary report for hello.exe on RTEMS 5 for SPARC is:
>>
>> inlined funcs   : 1412
>> total funcs : 1956
>>  % inline funcs : 72%
>>  total size : 174616
>> inline size : 81668
>>   % inline size : 46%
>>
>> This is a small application so it could be argued that skews the figures. A
>> large C/C++ application built with -O2 running on RTEMS 4.11 ARM reports the
>> inline usage as:
>>
>> inlined funcs   : 10370
>> total funcs : 17700
>>  % inline funcs : 58%
>>  total size : 3066240
>> inline size : 1249514
>>   % inline size : 40%
>>
>> This does not seem right to me.
>>
>> The report is new and there could be issues in the DWARF handling that feeds
>> this report however I am posting this to start a discussion on the topic of
>> inlining.
>>
>> I attach the report for hello.exe. The `-i` option generates the inline 
>> report.
>>
>> The first section is a summary showing the total number of functions in the
>> executable that have machine code and are flagged as inline. The report lists
>> the percentage of functions that are inlined and the percentage of machine 
>> code
>> that is inlined. The values seem high to me.
>>
>> The second table lists inline functions that are repeated sorted from the
>> largest foot print to the smallest. The first column the total size of 
>> machine
>> code in the executable and the second column the number of instances.
>>
>> The third table is the list of inline functions sorted from largest machine 
>> code
>> footprint to smallest. The second column are flags of which there is one. A 
>> `E`
>> indicates the inline function is also external which means the compiler has
>> created an external reference to this function, ie an address-of is being 
>> taken.
>> The third column is the address in the executable so you can take a look with
>> objdump at the machine code.
>>
>> We need to ask some important question in relation to inlining. It is cheap 
>> to
>> add and we all feel the code we add needs to be fast and needs to be inlined 
>> but
>> does it really need to be inlined?
>>
>> Some pieces of code do need to be inlined and the overhead is just that an
>> overhead, for example in the large C/C++ application there is a low level
>> volatile hardware write routing with close to 300 instances and a code size 
>> of
>> 10K. This code needs to be inlined for performance reasons but should the 
>> size
>> on average be 40 bytes, I doubt it.
>>
>> Can we be more judicious with our use of the inline keyword?
>>
>> Is the performance gain we really expect or is the actual overhead of a call
>> frame not worth saving?
>>
>> What are the real costs of inlining a piece of code? It adds size to the
>> executable and depending on the code being inlined it complicates coverage
>> analysis be adding extra branch points.
>>
>> The metrics to determine what should be inlined is complicated and I do not
>> think we have a suitable policy in place. I believe it is time we to create 
>> one.
>>
>> The issue is not limited to our code, gcc, newlib and libstdc++ seem to have
>> some code that should be looked at more closely. For example __udivmoddi4, 
>> and
>> __sprint_r.
>>
>> Chris
>>
>>
> 
> Hello Chris,
> 
> I just took a look at one of the first function in your list: __sprint_r
> 
> https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;a=blob;f=newlib/libc/stdio/vfprintf.c;h=c4bf2dbe31da64462ecccec97c8e901e4ffadd44;hb=HEAD#l403
> 
> As far as I can see, there is no explicit inline key word for that
> function. So in that case, the compiler decided that it would be a good
> idea to inline that function.

Thanks and yes. At this point in time I cannot tell what is happening and I am
not sure the tool is reporting accurate data, I need to investigate.

> I'm not sure whether I might just haven't seen it but is there a
> possibility to distinguish between functions that have been inlined by
> the compiler and ones that have been inlined due to the "inline" keyword
> without looking at every definition?

I am not sure. The DWARF data is complex and detailed and I view this initial
step into the area of using DWARF to perform static analysis of RTEMS
executables as green.

DWARF does provide declaration attributes. I need to review the DWARF data and
standard to determine if we can tell what is declared inline and what has been
inlined. I think it would be good to know.

> Did you t

Re: Inlined code

2018-08-05 Thread Sebastian Huber

Hello Chris,

this looks like a very interesting tool. The justification for an inline 
function must be done case by case.


On 05/08/18 04:00, Chris Johns wrote:

inlined repeats :
  19172   16 __sprint_r


This is a small function. GCC decided that it is worth inlining. With 
-O2 GCC doesn't care much about code size.



   19484 __udivmoddi4


We should figure out what is going on here with this __udivmoddi4.

I think uninteresting functions are the ones with inlined / repeats / 
opcode size <= 4 and repeats == 1.


--
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax : +49 89 189 47 41-09
E-Mail  : sebastian.hu...@embedded-brains.de
PGP : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: Inlined code

2018-08-05 Thread Chris Johns
On 06/08/2018 10:51, Chris Johns wrote:
> On 05/08/2018 19:39, Christian Mauderer wrote:
>> Am 05.08.2018 um 04:00 schrieb Chris Johns:
>>> Hi,
>>>
>>> I have been working on migrating covoar in the rtems-tools repo to DWARF. 
>>> The
>>> goal is remove objdump parsing and to get accurate details about the 
>>> functions
>>> being covered. This is an unfunded task.
>>>
>>> The work has resulted in a close examination of inlined code in RTEMS and 
>>> what I
>>> saw alarmed me so I have added a report to the rtems-exeinfo tool in 
>>> rtems-tools
>>> (the change is to be posted for review once I get the coverage tests 
>>> running).
>>>
>>> A summary report for hello.exe on RTEMS 5 for SPARC is:
>>>
>>> inlined funcs   : 1412
>>> total funcs : 1956
>>>  % inline funcs : 72%
>>>  total size : 174616
>>> inline size : 81668
>>>   % inline size : 46%
>>>
>>> This is a small application so it could be argued that skews the figures. A
>>> large C/C++ application built with -O2 running on RTEMS 4.11 ARM reports the
>>> inline usage as:
>>>
>>> inlined funcs   : 10370
>>> total funcs : 17700
>>>  % inline funcs : 58%
>>>  total size : 3066240
>>> inline size : 1249514
>>>   % inline size : 40%
>>>
>>> This does not seem right to me.
>>>
>>> The report is new and there could be issues in the DWARF handling that feeds
>>> this report however I am posting this to start a discussion on the topic of
>>> inlining.
>>>
>>> I attach the report for hello.exe. The `-i` option generates the inline 
>>> report.
>>>
>>> The first section is a summary showing the total number of functions in the
>>> executable that have machine code and are flagged as inline. The report 
>>> lists
>>> the percentage of functions that are inlined and the percentage of machine 
>>> code
>>> that is inlined. The values seem high to me.
>>>
>>> The second table lists inline functions that are repeated sorted from the
>>> largest foot print to the smallest. The first column the total size of 
>>> machine
>>> code in the executable and the second column the number of instances.
>>>
>>> The third table is the list of inline functions sorted from largest machine 
>>> code
>>> footprint to smallest. The second column are flags of which there is one. A 
>>> `E`
>>> indicates the inline function is also external which means the compiler has
>>> created an external reference to this function, ie an address-of is being 
>>> taken.
>>> The third column is the address in the executable so you can take a look 
>>> with
>>> objdump at the machine code.
>>>
>>> We need to ask some important question in relation to inlining. It is cheap 
>>> to
>>> add and we all feel the code we add needs to be fast and needs to be 
>>> inlined but
>>> does it really need to be inlined?
>>>
>>> Some pieces of code do need to be inlined and the overhead is just that an
>>> overhead, for example in the large C/C++ application there is a low level
>>> volatile hardware write routing with close to 300 instances and a code size 
>>> of
>>> 10K. This code needs to be inlined for performance reasons but should the 
>>> size
>>> on average be 40 bytes, I doubt it.
>>>
>>> Can we be more judicious with our use of the inline keyword?
>>>
>>> Is the performance gain we really expect or is the actual overhead of a call
>>> frame not worth saving?
>>>
>>> What are the real costs of inlining a piece of code? It adds size to the
>>> executable and depending on the code being inlined it complicates coverage
>>> analysis be adding extra branch points.
>>>
>>> The metrics to determine what should be inlined is complicated and I do not
>>> think we have a suitable policy in place. I believe it is time we to create 
>>> one.
>>>
>>> The issue is not limited to our code, gcc, newlib and libstdc++ seem to have
>>> some code that should be looked at more closely. For example __udivmoddi4, 
>>> and
>>> __sprint_r.
>>>
>>> Chris
>>>
>>>
>>
>> Hello Chris,
>>
>> I just took a look at one of the first function in your list: __sprint_r
>>
>> https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;a=blob;f=newlib/libc/stdio/vfprintf.c;h=c4bf2dbe31da64462ecccec97c8e901e4ffadd44;hb=HEAD#l403
>>
>> As far as I can see, there is no explicit inline key word for that
>> function. So in that case, the compiler decided that it would be a good
>> idea to inline that function.
> 
> Thanks and yes. At this point in time I cannot tell what is happening and I am
> not sure the tool is reporting accurate data, I need to investigate.

I have updated the tool and report to show which inline functions are:

 - inlined by compiler
 - declared inline and not inlined
 - declared inline and inlined

I have also fixed a quick hack I had where the size was the span from the low PC
to the high PC, this was wrong. Inlined code can be split and moved when
inlining creating a discontinuous address range. The size in the report is now
the number of machine code bytes.

The report will show any functions not i

[PATCH] libbsd.txt: Update version info and manpage links.

2018-08-05 Thread Christian Mauderer
The information is clearly outdated. This patch updates it to the latest
release.
---
 libbsd.txt | 50 --
 1 file changed, 28 insertions(+), 22 deletions(-)

diff --git a/libbsd.txt b/libbsd.txt
index c7a90f64..7945b6d1 100644
--- a/libbsd.txt
+++ b/libbsd.txt
@@ -5,7 +5,13 @@ RTEMS BSD Library Guide
 :numbered:
 :website: http://www.rtems.org/
 
-RTEMS uses FreeBSD 9.2 as the source of its TCP/IP and USB stacks.
+The libbsd makes FreeBSD subsystems like TCP/IP, USB, SD and some more usable
+for RTEMS. It tries to follow the FreeBSD development as close as possible and
+therefore is updated to the latest FreeBSD HEAD revision from time to time.
+To find out which version of FreeBSD is currently used as the base version for
+libbsd please take a look at the
+https://git.rtems.org/rtems-libbsd/log/freebsd-org[freebsd-org] submodule.
+
 This is a guide which captures information on the
 process of merging code from FreeBSD, building this library,
 RTEMS specific support files, and general guidelines on what
@@ -247,9 +253,9 @@ network_init(void)
 This performs the basic network stack initialization with a loopback interface.
 Further initialization must be done using the standard BSD network
 configuration commands
-http://www.freebsd.org/cgi/man.cgi?query=ifconfig&apropos=0&sektion=8&manpath=FreeBSD+9.2-RELEASE&arch=default&format=html[IFCONFIG(8)]
+http://www.freebsd.org/cgi/man.cgi?query=ifconfig&sektion=8[IFCONFIG(8)]
 using `rtems_bsd_command_ifconfig()` and
-http://www.freebsd.org/cgi/man.cgi?query=route&apropos=0&sektion=8&manpath=FreeBSD+9.2-RELEASE&arch=default&format=html[ROUTE(8)]
+http://www.freebsd.org/cgi/man.cgi?query=route&sektion=8[ROUTE(8)]
 using `rtems_bsd_command_route()`.  For an example please have a look at
 `testsuite/include/rtems/bsd/test/default-network-init.h`.
 
@@ -305,43 +311,43 @@ 
https://developer.apple.com/library/mac/documentation/Networking/Reference/DNSSe
 
 
http://www.opensource.apple.com/source/mDNSResponder/mDNSResponder-320.10/mDNSCore/mDNSEmbeddedAPI.h[mDNS]::
 Multi-Cast DNS
 
-http://www.freebsd.org/cgi/man.cgi?query=unix&sektion=4&apropos=0&manpath=FreeBSD+9.2-RELEASE[UNIX(4)]::
 UNIX-domain protocol family
+http://www.freebsd.org/cgi/man.cgi?query=unix&sektion=4[UNIX(4)]:: UNIX-domain 
protocol family
 
-http://www.freebsd.org/cgi/man.cgi?query=inet&sektion=4&apropos=0&manpath=FreeBSD+9.2-RELEASE[INET(4)]::
 Internet protocol family
+http://www.freebsd.org/cgi/man.cgi?query=inet&sektion=4[INET(4)]:: Internet 
protocol family
 
-http://www.freebsd.org/cgi/man.cgi?query=inet6&apropos=0&sektion=4&manpath=FreeBSD+9.2-RELEASE&arch=default&format=html[INET6(4)]::
 Internet protocol version 6 family
+http://www.freebsd.org/cgi/man.cgi?query=inet6&sektion=4[INET6(4)]:: Internet 
protocol version 6 family
 
-http://www.freebsd.org/cgi/man.cgi?query=tcp&apropos=0&sektion=4&manpath=FreeBSD+9.2-RELEASE&arch=default&format=html[TCP(4)]::
 Internet Transmission Control Protocol
+http://www.freebsd.org/cgi/man.cgi?query=tcp&sektion=4[TCP(4)]:: Internet 
Transmission Control Protocol
 
-http://www.freebsd.org/cgi/man.cgi?query=udp&apropos=0&sektion=4&manpath=FreeBSD+9.2-RELEASE&arch=default&format=html[UDP(4)]::
 Internet User Datagram Protocol
+http://www.freebsd.org/cgi/man.cgi?query=udp&sektion=4[UDP(4)]:: Internet User 
Datagram Protocol
 
-http://www.freebsd.org/cgi/man.cgi?query=route&sektion=4&apropos=0&manpath=FreeBSD+9.2-RELEASE[ROUTE(4)]::
 Kernel packet forwarding database
+http://www.freebsd.org/cgi/man.cgi?query=route&sektion=4[ROUTE(4)]:: Kernel 
packet forwarding database
 
-http://www.freebsd.org/cgi/man.cgi?query=bpf&apropos=0&sektion=4&manpath=FreeBSD+9.2-RELEASE&arch=default&format=html[BPF(4)]::
 Berkeley Packet Filter
+http://www.freebsd.org/cgi/man.cgi?query=bpf&sektion=4[BPF(4)]:: Berkeley 
Packet Filter
 
-http://www.freebsd.org/cgi/man.cgi?query=socket&apropos=0&sektion=2&manpath=FreeBSD+9.2-RELEASE&arch=default&format=html[SOCKET(2)]::
 Create an endpoint for communication
+http://www.freebsd.org/cgi/man.cgi?query=socket&sektion=2[SOCKET(2)]:: Create 
an endpoint for communication
 
-http://www.freebsd.org/cgi/man.cgi?query=kqueue&apropos=0&sektion=2&manpath=FreeBSD+9.2-RELEASE&arch=default&format=html[KQUEUE(2)]::
 Kernel event notification mechanism
+http://www.freebsd.org/cgi/man.cgi?query=kqueue&sektion=2[KQUEUE(2)]:: Kernel 
event notification mechanism
 
-http://www.freebsd.org/cgi/man.cgi?query=select&apropos=0&sektion=2&manpath=FreeBSD+9.2-RELEASE&arch=default&format=html[SELECT(2)]::
 Synchronous I/O multiplexing
+http://www.freebsd.org/cgi/man.cgi?query=select&sektion=2[SELECT(2)]:: 
Synchronous I/O multiplexing
 
-http://www.freebsd.org/cgi/man.cgi?query=poll&apropos=0&sektion=2&manpath=FreeBSD+9.2-RELEASE&arch=default&format=html[POLL(2)]::
 Synchronous I/O multiplexing
+http://www.freebsd.org/cgi/man.cgi?query=poll&sektion=2[POLL(2)]:: Synchronous 
I/O multiplexing
 
-http://www.freebsd.org/

Re: Inlined code

2018-08-05 Thread Chris Johns
On 06/08/2018 15:30, Sebastian Huber wrote:
> Hello Chris,
> 
> this looks like a very interesting tool. The justification for an inline
> function must be done case by case.

I agree. I think a workable pattern will appear and my hope is this tool can be
taught what is OK and what is not.

I hope Joel discusses coverage analysis. This is another factor we need to
consider.

> 
> On 05/08/18 04:00, Chris Johns wrote:
>> inlined repeats :
>>   19172   16 __sprint_r
> 
> This is a small function. GCC decided that it is worth inlining. With -O2 GCC
> doesn't care much about code size.

Please have a look at the updated report. Things have changed with better
accounting of the size. I have dumped all the data here:

 https://ftp.rtems.org/pub/rtems/people/chrisj/coverage/inlines/

It has the inlined report, the DWARF dump and the assembler.

I am not sure about the inline function addresses shown. I think I need to see
what is happening here a little more. For example for __sprint_r DWARF range
data has a class of 'sec_offset' which means I may need to get the section the
code lives in so the section offsets mean something. I think the current
addresses are wrong.

> 
>>    1948    4 __udivmoddi4
> 
> We should figure out what is going on here with this __udivmoddi4.
> 

Yeap.

> I think uninteresting functions are the ones with inlined / repeats / opcode
> size <= 4 and repeats == 1.

I think this may depend on the machine work size. That data is present in the
DWARF CU info.

Chris
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: [PATCH] libbsd.txt: Update version info and manpage links.

2018-08-05 Thread Sebastian Huber

On 06/08/18 07:44, Christian Mauderer wrote:

The information is clearly outdated. This patch updates it to the latest
release.


Looks good. Please check in.

--
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax : +49 89 189 47 41-09
E-Mail  : sebastian.hu...@embedded-brains.de
PGP : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: Inlined code

2018-08-05 Thread Christian Mauderer
Am 06.08.2018 um 07:31 schrieb Chris Johns:
> On 06/08/2018 10:51, Chris Johns wrote:
>> On 05/08/2018 19:39, Christian Mauderer wrote:
>>> Am 05.08.2018 um 04:00 schrieb Chris Johns:
 Hi,

 I have been working on migrating covoar in the rtems-tools repo to DWARF. 
 The
 goal is remove objdump parsing and to get accurate details about the 
 functions
 being covered. This is an unfunded task.

 The work has resulted in a close examination of inlined code in RTEMS and 
 what I
 saw alarmed me so I have added a report to the rtems-exeinfo tool in 
 rtems-tools
 (the change is to be posted for review once I get the coverage tests 
 running).

 A summary report for hello.exe on RTEMS 5 for SPARC is:

 inlined funcs   : 1412
 total funcs : 1956
  % inline funcs : 72%
  total size : 174616
 inline size : 81668
   % inline size : 46%

 This is a small application so it could be argued that skews the figures. A
 large C/C++ application built with -O2 running on RTEMS 4.11 ARM reports 
 the
 inline usage as:

 inlined funcs   : 10370
 total funcs : 17700
  % inline funcs : 58%
  total size : 3066240
 inline size : 1249514
   % inline size : 40%

 This does not seem right to me.

 The report is new and there could be issues in the DWARF handling that 
 feeds
 this report however I am posting this to start a discussion on the topic of
 inlining.

 I attach the report for hello.exe. The `-i` option generates the inline 
 report.

 The first section is a summary showing the total number of functions in the
 executable that have machine code and are flagged as inline. The report 
 lists
 the percentage of functions that are inlined and the percentage of machine 
 code
 that is inlined. The values seem high to me.

 The second table lists inline functions that are repeated sorted from the
 largest foot print to the smallest. The first column the total size of 
 machine
 code in the executable and the second column the number of instances.

 The third table is the list of inline functions sorted from largest 
 machine code
 footprint to smallest. The second column are flags of which there is one. 
 A `E`
 indicates the inline function is also external which means the compiler has
 created an external reference to this function, ie an address-of is being 
 taken.
 The third column is the address in the executable so you can take a look 
 with
 objdump at the machine code.

 We need to ask some important question in relation to inlining. It is 
 cheap to
 add and we all feel the code we add needs to be fast and needs to be 
 inlined but
 does it really need to be inlined?

 Some pieces of code do need to be inlined and the overhead is just that an
 overhead, for example in the large C/C++ application there is a low level
 volatile hardware write routing with close to 300 instances and a code 
 size of
 10K. This code needs to be inlined for performance reasons but should the 
 size
 on average be 40 bytes, I doubt it.

 Can we be more judicious with our use of the inline keyword?

 Is the performance gain we really expect or is the actual overhead of a 
 call
 frame not worth saving?

 What are the real costs of inlining a piece of code? It adds size to the
 executable and depending on the code being inlined it complicates coverage
 analysis be adding extra branch points.

 The metrics to determine what should be inlined is complicated and I do not
 think we have a suitable policy in place. I believe it is time we to 
 create one.

 The issue is not limited to our code, gcc, newlib and libstdc++ seem to 
 have
 some code that should be looked at more closely. For example __udivmoddi4, 
 and
 __sprint_r.

 Chris


>>>
>>> Hello Chris,
>>>
>>> I just took a look at one of the first function in your list: __sprint_r
>>>
>>> https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;a=blob;f=newlib/libc/stdio/vfprintf.c;h=c4bf2dbe31da64462ecccec97c8e901e4ffadd44;hb=HEAD#l403
>>>
>>> As far as I can see, there is no explicit inline key word for that
>>> function. So in that case, the compiler decided that it would be a good
>>> idea to inline that function.
>>
>> Thanks and yes. At this point in time I cannot tell what is happening and I 
>> am
>> not sure the tool is reporting accurate data, I need to investigate.
> 
> I have updated the tool and report to show which inline functions are:
> 
>  - inlined by compiler
>  - declared inline and not inlined
>  - declared inline and inlined
> 
> I have also fixed a quick hack I had where the size was the span from the low 
> PC
> to the high PC, 

[PATCH] rfs: Remove erroneous call of rtems_disk_release()

2018-08-05 Thread Sebastian Huber
The function rtems_rfs_buffer_sync() erroneously calls
rtems_disk_release(). This screws up the reference counting of the disk.

Update #3484.
---
 cpukit/libfs/src/rfs/rtems-rfs-buffer.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/cpukit/libfs/src/rfs/rtems-rfs-buffer.c 
b/cpukit/libfs/src/rfs/rtems-rfs-buffer.c
index 33beb54700..4a66840b17 100644
--- a/cpukit/libfs/src/rfs/rtems-rfs-buffer.c
+++ b/cpukit/libfs/src/rfs/rtems-rfs-buffer.c
@@ -397,7 +397,6 @@ rtems_rfs_buffer_sync (rtems_rfs_file_system* fs)
   rtems_status_text (sc));
 result = EIO;
   }
-  rtems_disk_release (fs->disk);
 #else
   if (fsync (fs->device) < 0)
   {
-- 
2.13.7

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


Re: [PATCH] rfs: Remove erroneous call of rtems_disk_release()

2018-08-05 Thread Sebastian Huber

Hello Chris,

this needs a back port to 4.11 and 4.10.

--
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax : +49 89 189 47 41-09
E-Mail  : sebastian.hu...@embedded-brains.de
PGP : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: Inlined code

2018-08-05 Thread Chris Johns
On 06/08/2018 16:12, Christian Mauderer wrote:
> Am 06.08.2018 um 07:31 schrieb Chris Johns:
>> On 06/08/2018 10:51, Chris Johns wrote:
>>> On 05/08/2018 19:39, Christian Mauderer wrote:
 Am 05.08.2018 um 04:00 schrieb Chris Johns:
> Hi,
>
> I have been working on migrating covoar in the rtems-tools repo to DWARF. 
> The
> goal is remove objdump parsing and to get accurate details about the 
> functions
> being covered. This is an unfunded task.
>
> The work has resulted in a close examination of inlined code in RTEMS and 
> what I
> saw alarmed me so I have added a report to the rtems-exeinfo tool in 
> rtems-tools
> (the change is to be posted for review once I get the coverage tests 
> running).
>
> A summary report for hello.exe on RTEMS 5 for SPARC is:
>
> inlined funcs   : 1412
> total funcs : 1956
>  % inline funcs : 72%
>  total size : 174616
> inline size : 81668
>   % inline size : 46%
>
> This is a small application so it could be argued that skews the figures. 
> A
> large C/C++ application built with -O2 running on RTEMS 4.11 ARM reports 
> the
> inline usage as:
>
> inlined funcs   : 10370
> total funcs : 17700
>  % inline funcs : 58%
>  total size : 3066240
> inline size : 1249514
>   % inline size : 40%
>
> This does not seem right to me.
>
> The report is new and there could be issues in the DWARF handling that 
> feeds
> this report however I am posting this to start a discussion on the topic 
> of
> inlining.
>
> I attach the report for hello.exe. The `-i` option generates the inline 
> report.
>
> The first section is a summary showing the total number of functions in 
> the
> executable that have machine code and are flagged as inline. The report 
> lists
> the percentage of functions that are inlined and the percentage of 
> machine code
> that is inlined. The values seem high to me.
>
> The second table lists inline functions that are repeated sorted from the
> largest foot print to the smallest. The first column the total size of 
> machine
> code in the executable and the second column the number of instances.
>
> The third table is the list of inline functions sorted from largest 
> machine code
> footprint to smallest. The second column are flags of which there is one. 
> A `E`
> indicates the inline function is also external which means the compiler 
> has
> created an external reference to this function, ie an address-of is being 
> taken.
> The third column is the address in the executable so you can take a look 
> with
> objdump at the machine code.
>
> We need to ask some important question in relation to inlining. It is 
> cheap to
> add and we all feel the code we add needs to be fast and needs to be 
> inlined but
> does it really need to be inlined?
>
> Some pieces of code do need to be inlined and the overhead is just that an
> overhead, for example in the large C/C++ application there is a low level
> volatile hardware write routing with close to 300 instances and a code 
> size of
> 10K. This code needs to be inlined for performance reasons but should the 
> size
> on average be 40 bytes, I doubt it.
>
> Can we be more judicious with our use of the inline keyword?
>
> Is the performance gain we really expect or is the actual overhead of a 
> call
> frame not worth saving?
>
> What are the real costs of inlining a piece of code? It adds size to the
> executable and depending on the code being inlined it complicates coverage
> analysis be adding extra branch points.
>
> The metrics to determine what should be inlined is complicated and I do 
> not
> think we have a suitable policy in place. I believe it is time we to 
> create one.
>
> The issue is not limited to our code, gcc, newlib and libstdc++ seem to 
> have
> some code that should be looked at more closely. For example 
> __udivmoddi4, and
> __sprint_r.
>
> Chris
>
>

 Hello Chris,

 I just took a look at one of the first function in your list: __sprint_r

 https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;a=blob;f=newlib/libc/stdio/vfprintf.c;h=c4bf2dbe31da64462ecccec97c8e901e4ffadd44;hb=HEAD#l403

 As far as I can see, there is no explicit inline key word for that
 function. So in that case, the compiler decided that it would be a good
 idea to inline that function.
>>>
>>> Thanks and yes. At this point in time I cannot tell what is happening and I 
>>> am
>>> not sure the tool is reporting accurate data, I need to investigate.
>>
>> I have updated the tool and report to show which inline functions ar