PING: [PATCH] benchtests: Add -f/--functions argument

Message ID CAMe9rOqid5ksKa=tt5fu43Oox5khKF=+ESPcb5XF+s_wxBEkhg@mail.gmail.com
State New, archived
Headers

Commit Message

H.J. Lu June 12, 2018, 2:29 p.m. UTC
  On Thu, May 24, 2018 at 10:40 AM, H.J. Lu <hongjiu.lu@intel.com> wrote:
> On x86-64, there may be multiple IFUNC implementations for a given
> function.  But we may be only interested in a subset of them.  This
> patch adds -f/--functions argument to compare a subset of IFUNC
> implementations.
>
> Any comments?
>
>
> H.J.
> ---
>         * benchtests/scripts/compare_strings.py (process_results): Add
>         funcs argument.  Compare only functions which are selected.
>         (main): Check if base function is among selected functions.
>         Pass selected functions to process_results.
>         (__main__): Add -f/--functions argument.

I rebased my patch against the current master.

Any comments, objections?
  

Comments

Victor Rodriguez June 12, 2018, 2:45 p.m. UTC | #1
On Tue, Jun 12, 2018 at 9:29 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, May 24, 2018 at 10:40 AM, H.J. Lu <hongjiu.lu@intel.com> wrote:
>> On x86-64, there may be multiple IFUNC implementations for a given
>> function.  But we may be only interested in a subset of them.  This
>> patch adds -f/--functions argument to compare a subset of IFUNC
>> implementations.
>>
>> Any comments?
>>
>>
>> H.J.
>> ---
>>         * benchtests/scripts/compare_strings.py (process_results): Add
>>         funcs argument.  Compare only functions which are selected.
>>         (main): Check if base function is among selected functions.
>>         Pass selected functions to process_results.
>>         (__main__): Add -f/--functions argument.
>
> I rebased my patch against the current master.
>
> Any comments, objections?
>

+1 i like the idea funcs argument.

> --
> H.J.
  
Siddhesh Poyarekar June 12, 2018, 4:05 p.m. UTC | #2
On 06/12/2018 07:59 PM, H.J. Lu wrote:
> On Thu, May 24, 2018 at 10:40 AM, H.J. Lu <hongjiu.lu@intel.com> wrote:
>> On x86-64, there may be multiple IFUNC implementations for a given
>> function.  But we may be only interested in a subset of them.  This
>> patch adds -f/--functions argument to compare a subset of IFUNC
>> implementations.
>>
>> Any comments?
>>
>>
>> H.J.
>> ---
>>          * benchtests/scripts/compare_strings.py (process_results): Add
>>          funcs argument.  Compare only functions which are selected.
>>          (main): Check if base function is among selected functions.
>>          Pass selected functions to process_results.
>>          (__main__): Add -f/--functions argument.
> 
> I rebased my patch against the current master.
> 
> Any comments, objections?

Thanks, looks OK to me.  I kinda dislike the idea of 7 arguments to 
process_results but that probably is a separate cleanup.

Siddhesh
  

Patch

From cfed5f5b78c22eb7927e68a603fa532c73183c52 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 24 May 2018 10:12:26 -0700
Subject: [PATCH] benchtests: Add -f/--functions argument

On x86-64, there may be multiple IFUNC implementations for a given
function.  But we may be only interested in a subset of them.  This
patch adds -f/--functions argument to compare a subset of IFUNC
implementations.

	* benchtests/scripts/compare_strings.py (process_results): Add
	funcs argument.  Compare only functions which are selected.
	(main): Check if base function is among selected functions.
	Pass selected functions to process_results.
	(__main__): Add -f/--functions argument.
---
 benchtests/scripts/compare_strings.py | 52 +++++++++++++++++++++------
 1 file changed, 42 insertions(+), 10 deletions(-)

diff --git a/benchtests/scripts/compare_strings.py b/benchtests/scripts/compare_strings.py
index ddce84a3ac..e3ad8ff058 100755
--- a/benchtests/scripts/compare_strings.py
+++ b/benchtests/scripts/compare_strings.py
@@ -82,19 +82,41 @@  def draw_graph(f, v, ifuncs, results):
     pylab.savefig('%s-%s.png' % (f, v), bbox_inches='tight')
 
 
-def process_results(results, attrs, base_func, graph, no_diff, no_header):
+def process_results(results, attrs, funcs, base_func, graph, no_diff, no_header):
     """ Process results and print them
 
     Args:
         results: JSON dictionary of results
         attrs: Attributes that form the test criteria
+        funcs: Functions that are selected
     """
 
     for f in results['functions'].keys():
 
         v = results['functions'][f]['bench-variant']
 
+        selected = {}
+        index = 0
         base_index = 0
+        if funcs:
+            ifuncs = []
+            first_func = True
+            for i in results['functions'][f]['ifuncs']:
+                if i in funcs:
+                    if first_func:
+                        base_index = index
+                        first_func = False
+                    selected[index] = 1
+                    ifuncs.append(i)
+                else:
+                    selected[index] = 0
+                index += 1
+        else:
+            ifuncs = results['functions'][f]['ifuncs']
+            for i in ifuncs:
+                selected[index] = 1
+                index += 1
+
         if base_func:
             try:
                 base_index = results['functions'][f]['ifuncs'].index(base_func)
@@ -106,7 +128,7 @@  def process_results(results, attrs, base_func, graph, no_diff, no_header):
         if not no_header:
             print('Function: %s' % f)
             print('Variant: %s' % v)
-            print("%36s%s" % (' ', '\t'.join(results['functions'][f]['ifuncs'])))
+            print("%36s%s" % (' ', '\t'.join(ifuncs)))
             print("=" * 120)
 
         graph_res = {}
@@ -122,13 +144,14 @@  def process_results(results, attrs, base_func, graph, no_diff, no_header):
             sys.stdout.write('%36s: ' % key)
             graph_res[key] = res['timings']
             for t in res['timings']:
-                sys.stdout.write ('%12.2f' % t)
-                if not no_diff:
-                    if i != base_index:
-                        base = res['timings'][base_index]
-                        diff = (base - t) * 100 / base
-                        sys.stdout.write (' (%6.2f%%)' % diff)
-                sys.stdout.write('\t')
+                if selected[i]:
+                    sys.stdout.write ('%12.2f' % t)
+                    if not no_diff:
+                        if i != base_index:
+                            base = res['timings'][base_index]
+                            diff = (base - t) * 100 / base
+                            sys.stdout.write (' (%6.2f%%)' % diff)
+                    sys.stdout.write('\t')
                 i = i + 1
             print('')
 
@@ -147,9 +170,16 @@  def main(args):
     schema_filename = args.schema
     base_func = args.base
     attrs = args.attributes.split(',')
+    if args.functions:
+        funcs = args.functions.split(',')
+        if base_func and not base_func in funcs:
+            print('Baseline function (%s) not found.' % base_func)
+            sys.exit(os.EX_DATAERR)
+    else:
+        funcs = None
 
     results = parse_file(args.input, args.schema)
-    process_results(results, attrs, base_func, args.graph, args.no_diff, args.no_header)
+    process_results(results, attrs, funcs, base_func, args.graph, args.no_diff, args.no_header)
     return os.EX_OK
 
 
@@ -166,6 +196,8 @@  if __name__ == '__main__':
                         help='Schema file to validate the result file.')
 
     # Optional arguments.
+    parser.add_argument('-f', '--functions',
+                        help='Comma separated list of functions.')
     parser.add_argument('-b', '--base',
                         help='IFUNC variant to set as baseline.')
     parser.add_argument('-g', '--graph', action='store_true',
-- 
2.17.1