RE: [PATCH v2] AArch64: Add LUTI ACLE for SVE2

[Vladimir Miloserdov]

Hi Kyrill,

Thanks for your prompt review again!

>It looks like __ARM_FEATURE_LUT should guard the Advanced SIMD intrinsics for 
>LUTI at least.
>Therefore we should add this macro definition only once those are implemented 
>as well. So I’d remove this hunk.
I'm working on adding those Advanced SIMD intrinsics now.  The flag should be 
added by either SVE or AdvSIMD LUTI intrinsics patch from my understanding. I 
think this one is first to go in, so should we leave it here?

>Since this patch depends on Andrew’s feature flags rework shouldn’t this hunk 
>look like:
>AARCH64_HAVE_ISA (LUT)
>Instead?
Yes, looks like I was using an older revision of Andrew's work. I'll wait for 
it to be merged and update the patch then.

>+# Return 1 if this is an AArch64 target supporting LUT (Lookup table)
>I don’t see this effective target check used anywhere, do we need to add it?
>I guess I don’t mind it for future use, but just checking that it was 
>deliberate.
It is deliberate - I thought it may be useful to have for future use.

BR,
- Vladimir Miloserdov

RE: [PATCH][contrib]: support json output from check_GNU_style_lib.py

[Vladimir Miloserdov]

Hi Tamar,

A few suggestions below.

>diff --git a/contrib/check_GNU_style_lib.py b/contrib/check_GNU_style_lib.py 
>index 
>>6dbe4b53559c63d2e0276d0ff88619cd2f7f8e06..ab21ed4607593668ab95f24715295a41ac7d8>a21
> 100755
>--- a/contrib/check_GNU_style_lib.py
>+++ b/contrib/check_GNU_style_lib.py
>@@ -29,6 +29,7 @@
> import sys
> import re
> import unittest
>+import json
 
> def import_pip3(*args):
> missing=[]
>@@ -317,6 +318,33 @@ def check_GNU_style_file(file, format):
> else:
> print('%d error(s) written to %s file.' % (len(errors), f))
> exit(1)
>+elif format == 'json':
>+fn = lambda x: x.error_message
>+i = 1
>+result = []
>+for (k, errors) in groupby(sorted(errors, key = fn), fn):
>+errors = list(errors)
>+entry = {}
>+entry['type'] = i
>+entry['msg'] = k
>+entry['count'] = len(errors)
>+i += 1
>+errlines = []
>+for e in errors:
>+locs = e.error_location ().split(':')
>+errlines.append({ "file": locs[0]
>+, "row": int(locs[1])
>+, "column": int(locs[2])
>+, "err": e.console_error })
>+entry['errors'] = errlines
>+result.append(entry)
>+
>+if len(errors) == 0:
>+exit(0)
>+else:
>+json_string = json.dumps(result)
>+print(json_string)
>+exit(1)
> else:
> assert False

Might be a good idea to rename "fn" -> "get_err", "i" -> "error_type_counter", 
"k" -> "error_message", "errors" -> "grouped_errors" to make it easier to 
understand. 

You can also simplify "entry" construction like this:
entry = {
'type': error_type_counter,
'msg': error_message,
'count': len(grouped_errors)
}

BR,
- Vladimir Miloserdov