Re: [Mesa-dev] [PATCH] i965: annotate brw_oa.py's --header and --code as required

2018-03-28 Thread Eero Tamminen 

Hi,

On 20.03.2018 19:06, Dylan Baker wrote:

Quoting Emil Velikov (2018-03-20 09:29:00)
[snip]

  gens = []
  for xml_file in args.xml_files:
@@ -617,7 +610,7 @@ def main():
  
  """))
  
-c("#include \"" + os.path.basename(args.header) + "\"")

+c("#include \"" + os.path.basename(header_file) + "\"")


You're calling os.path.basename on a file object, which isn't valid. This should
still be args.header.


One could also use .name.


- Eero

  
  c(textwrap.dedent("""\

  #include "brw_context.h"

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] i965: annotate brw_oa.py's --header and --code as required

2018-03-20 Thread Emil Velikov 
On 20 March 2018 at 17:06, Lionel Landwerlin
 wrote:
> On 20/03/18 16:29, Emil Velikov wrote:
>>
>> From: Emil Velikov 
>>
>> As of earlier commit, the --header was made a hard requirement when
>> using --code.
>>
>> Hence - annotate both as required and drop a few no longer needed
>> checks.
>>
>> Fixes: 035cc7a12dc0 ("i965: perf: reduce i965 binary size")
>> Cc: Lionel Landwerlin 
>> Signed-off-by: Emil Velikov 
>> ---
>> Tad easier to read with git show -w
>> ---
>>   src/mesa/drivers/dri/i965/brw_oa.py | 37
>> +++--
>>   1 file changed, 15 insertions(+), 22 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_oa.py
>> b/src/mesa/drivers/dri/i965/brw_oa.py
>> index 63db28bba97..4719b4c01c8 100644
>> --- a/src/mesa/drivers/dri/i965/brw_oa.py
>> +++ b/src/mesa/drivers/dri/i965/brw_oa.py
>> @@ -32,19 +32,16 @@ c_file = None
>>   _c_indent = 0
>> def c(*args):
>> -if c_file:
>> -code = ' '.join(map(str,args))
>> -for line in code.splitlines():
>> -text = ''.rjust(_c_indent) + line
>> -c_file.write(text.rstrip() + "\n")
>> +code = ' '.join(map(str,args))
>> +for line in code.splitlines():
>> +text = ''.rjust(_c_indent) + line
>> +c_file.write(text.rstrip() + "\n")
>> # indented, but no trailing newline...
>>   def c_line_start(code):
>> -if c_file:
>> -c_file.write(''.rjust(_c_indent) + code)
>> +c_file.write(''.rjust(_c_indent) + code)
>>   def c_raw(code):
>> -if c_file:
>> -c_file.write(code)
>> +c_file.write(code)
>> def c_indent(n):
>>   global _c_indent
>> @@ -57,11 +54,10 @@ header_file = None
>>   _h_indent = 0
>> def h(*args):
>> -if header_file:
>> -code = ' '.join(map(str,args))
>> -for line in code.splitlines():
>> -text = ''.rjust(_h_indent) + line
>> -header_file.write(text.rstrip() + "\n")
>> +code = ' '.join(map(str,args))
>> +for line in code.splitlines():
>> +text = ''.rjust(_h_indent) + line
>> +header_file.write(text.rstrip() + "\n")
>> def h_indent(n):
>>   global _c_indent
>> @@ -556,17 +552,14 @@ def main():
>>   global header_file
>> parser = argparse.ArgumentParser()
>> -parser.add_argument("--header", help="Header file to write")
>> -parser.add_argument("--code", help="C file to write")
>> +parser.add_argument("--header", help="Header file to write",
>> required=True)
>> +parser.add_argument("--code", help="C file to write", required=True)
>>   parser.add_argument("xml_files", nargs='+', help="List of xml
>> metrics files to process")
>> args = parser.parse_args()
>>   -if args.header:
>> -header_file = open(args.header, 'w')
>> -
>> -if args.code:
>> -c_file = open(args.code, 'w')
>> +header_file = open(args.header, 'w')
>> +c_file = open(args.code, 'w')
>> gens = []
>>   for xml_file in args.xml_files:
>> @@ -617,7 +610,7 @@ def main():
>> """))
>>   -c("#include \"" + os.path.basename(args.header) + "\"")
>> +c("#include \"" + os.path.basename(header_file) + "\"")
>
>
> basename() on a file object doesn't work.
> With that fixed :
>
> Reviewed-by: Lionel Landwerlin 
>
Right. Fixed and pushed.

Thank you
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] i965: annotate brw_oa.py's --header and --code as required

2018-03-20 Thread Lionel Landwerlin 

On 20/03/18 16:29, Emil Velikov wrote:

From: Emil Velikov 

As of earlier commit, the --header was made a hard requirement when
using --code.

Hence - annotate both as required and drop a few no longer needed
checks.

Fixes: 035cc7a12dc0 ("i965: perf: reduce i965 binary size")
Cc: Lionel Landwerlin 
Signed-off-by: Emil Velikov 
---
Tad easier to read with git show -w
---
  src/mesa/drivers/dri/i965/brw_oa.py | 37 +++--
  1 file changed, 15 insertions(+), 22 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_oa.py 
b/src/mesa/drivers/dri/i965/brw_oa.py
index 63db28bba97..4719b4c01c8 100644
--- a/src/mesa/drivers/dri/i965/brw_oa.py
+++ b/src/mesa/drivers/dri/i965/brw_oa.py
@@ -32,19 +32,16 @@ c_file = None
  _c_indent = 0
  
  def c(*args):

-if c_file:
-code = ' '.join(map(str,args))
-for line in code.splitlines():
-text = ''.rjust(_c_indent) + line
-c_file.write(text.rstrip() + "\n")
+code = ' '.join(map(str,args))
+for line in code.splitlines():
+text = ''.rjust(_c_indent) + line
+c_file.write(text.rstrip() + "\n")
  
  # indented, but no trailing newline...

  def c_line_start(code):
-if c_file:
-c_file.write(''.rjust(_c_indent) + code)
+c_file.write(''.rjust(_c_indent) + code)
  def c_raw(code):
-if c_file:
-c_file.write(code)
+c_file.write(code)
  
  def c_indent(n):

  global _c_indent
@@ -57,11 +54,10 @@ header_file = None
  _h_indent = 0
  
  def h(*args):

-if header_file:
-code = ' '.join(map(str,args))
-for line in code.splitlines():
-text = ''.rjust(_h_indent) + line
-header_file.write(text.rstrip() + "\n")
+code = ' '.join(map(str,args))
+for line in code.splitlines():
+text = ''.rjust(_h_indent) + line
+header_file.write(text.rstrip() + "\n")
  
  def h_indent(n):

  global _c_indent
@@ -556,17 +552,14 @@ def main():
  global header_file
  
  parser = argparse.ArgumentParser()

-parser.add_argument("--header", help="Header file to write")
-parser.add_argument("--code", help="C file to write")
+parser.add_argument("--header", help="Header file to write", required=True)
+parser.add_argument("--code", help="C file to write", required=True)
  parser.add_argument("xml_files", nargs='+', help="List of xml metrics files to 
process")
  
  args = parser.parse_args()
  
-if args.header:

-header_file = open(args.header, 'w')
-
-if args.code:
-c_file = open(args.code, 'w')
+header_file = open(args.header, 'w')
+c_file = open(args.code, 'w')
  
  gens = []

  for xml_file in args.xml_files:
@@ -617,7 +610,7 @@ def main():
  
  """))
  
-c("#include \"" + os.path.basename(args.header) + "\"")

+c("#include \"" + os.path.basename(header_file) + "\"")


basename() on a file object doesn't work.
With that fixed :

Reviewed-by: Lionel Landwerlin 

Thanks for this cleanup!

  
  c(textwrap.dedent("""\

  #include "brw_context.h"



___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] i965: annotate brw_oa.py's --header and --code as required

2018-03-20 Thread Dylan Baker 
Quoting Emil Velikov (2018-03-20 09:29:00)
[snip]
>  gens = []
>  for xml_file in args.xml_files:
> @@ -617,7 +610,7 @@ def main():
>  
>  """))
>  
> -c("#include \"" + os.path.basename(args.header) + "\"")
> +c("#include \"" + os.path.basename(header_file) + "\"")

You're calling os.path.basename on a file object, which isn't valid. This should
still be args.header.

>  
>  c(textwrap.dedent("""\
>  #include "brw_context.h"
> -- 
> 2.16.0
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


signature.asc
Description: signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev