[contrib] Extend and improve validate_failures.py
This patch series extends and improves validate_failures.py script to provide a powerful tool to handle DejaGnu test results in automated CI environment. Linaro TCWG uses validate_failures.py to ... - compare test results without human oversight, - detect unexpected FAILs vs baseline, - detect unexpected PASSes vs baseline, - automatically detect flaky tests, - create lists of expected failures and flaky tests, see [1]. [1] https://ci.linaro.org/job/tcwg_gcc_check--master-arm-build/lastSuccessfulBuild/artifact/artifacts/sumfiles/xfails.xfail/*view*/
[PATCH 01/12] [contrib] validate_failures.py: Avoid testsuite aliasing
This patch adds tracking of current testsuite "tool" and "exp"
to the processing of .sum files. This avoids aliasing between
tests from different testsuites with same name+description.
E.g., this is necessary for testsuite/c-c++-common, which is ran
for both gcc and g++ "tools".
This patch changes manifest format from ...
FAIL: gcc_test
FAIL: g++_test
... to ...
=== gcc tests ===
Running gcc/foo.exp ...
FAIL: gcc_test
=== gcc Summary ==
=== g++ tests ===
Running g++/bar.exp ...
FAIL: g++_test
=== g++ Summary ==
.
The new format uses same formatting as DejaGnu's .sum files
to specify which "tool" and "exp" the test belongs to.
---
.../testsuite-management/validate_failures.py | 137 +++---
1 file changed, 115 insertions(+), 22 deletions(-)
diff --git a/contrib/testsuite-management/validate_failures.py
b/contrib/testsuite-management/validate_failures.py
index 43d9d50af8d..94ba2e58b51 100755
--- a/contrib/testsuite-management/validate_failures.py
+++ b/contrib/testsuite-management/validate_failures.py
@@ -64,6 +64,16 @@ import sys
_VALID_TEST_RESULTS = [ 'FAIL', 'UNRESOLVED', 'XPASS', 'ERROR' ]
_VALID_TEST_RESULTS_REX = re.compile("%s" % "|".join(_VALID_TEST_RESULTS))
+# Formats of .sum file sections
+_TOOL_LINE_FORMAT = '\t\t=== %s tests ===\n'
+_EXP_LINE_FORMAT = '\nRunning %s ...\n'
+_SUMMARY_LINE_FORMAT = '\n\t\t=== %s Summary ===\n'
+
+# ... and their compiled regexs.
+_TOOL_LINE_REX = re.compile('^\t\t=== (.*) tests ===\n')
+_EXP_LINE_REX = re.compile('^Running (.*\.exp) \.\.\.\n')
+_SUMMARY_LINE_REX = re.compile('^\t\t=== (.*) Summary ===\n')
+
# Subdirectory of srcdir in which to find the manifest file.
_MANIFEST_SUBDIR = 'contrib/testsuite-management'
@@ -111,9 +121,11 @@ class TestResult(object):
ordinal: Monotonically increasing integer.
It is used to keep results for one .exp file sorted
by the order the tests were run.
+tool: Top-level testsuite name (aka "tool" in DejaGnu parlance) of the
test.
+exp: Name of .exp testsuite file.
"""
- def __init__(self, summary_line, ordinal=-1):
+ def __init__(self, summary_line, ordinal, tool, exp):
try:
(self.attrs, summary_line) = SplitAttributesFromSummaryLine(summary_line)
try:
@@ -125,6 +137,12 @@ class TestResult(object):
print('Failed to parse summary line: "%s"' % summary_line)
raise
self.ordinal = ordinal
+ if tool == None or exp == None:
+# .sum file seem to be broken. There was no "tool" and/or "exp"
+# lines preceding this result.
+raise
+ self.tool = tool
+ self.exp = exp
except ValueError:
Error('Cannot parse summary line "%s"' % summary_line)
@@ -133,14 +151,27 @@ class TestResult(object):
self.state, summary_line, self))
def __lt__(self, other):
-return (self.name < other.name or
-(self.name == other.name and self.ordinal < other.ordinal))
+if (self.tool != other.tool):
+ return self.tool < other.tool
+if (self.exp != other.exp):
+ return self.exp < other.exp
+if (self.name != other.name):
+ return self.name < other.name
+return self.ordinal < other.ordinal
def __hash__(self):
-return hash(self.state) ^ hash(self.name) ^ hash(self.description)
-
+return (hash(self.state) ^ hash(self.tool) ^ hash(self.exp)
+^ hash(self.name) ^ hash(self.description))
+
+ # Note that we don't include "attrs" in this comparison. This means that
+ # result entries "FAIL: test" and "flaky | FAIL: test" are considered
+ # the same. Therefore the ResultSet will preserve only the first occurence.
+ # In practice this means that flaky entries should preceed expected fails
+ # entries.
def __eq__(self, other):
return (self.state == other.state and
+self.tool == other.tool and
+self.exp == other.exp and
self.name == other.name and
self.description == other.description)
@@ -174,6 +205,43 @@ class TestResult(object):
return now > expiration_date
+class ResultSet(set):
+ """Describes a set of DejaGNU test results.
+ This set can be read in from .sum files or emitted as a manifest.
+
+ Attributes:
+current_tool: Name of the current top-level DejaGnu testsuite.
+current_exp: Name of the current .exp testsuite file.
+ """
+
+ def __init__(self):
+super().__init__()
+self.ResetToolExp()
+
+ def ResetToolExp(self):
+self.current_tool = None
+self.current_exp = None
+
+ def MakeTestResult(self, summary_line, ordinal=-1):
+return TestResult(summary_line, ordinal,
+ self.current_tool, self.current_exp)
+
+ def Print(self, outfile=sys.stdout):
+current_tool = None
+current_exp = None
+
+for result in sorted(self):
+ if current_tool != result.tool:
+current_tool = result.tool
+outfile.write(_TOO
[PATCH 04/12] [contrib] validate_failures.py: Simplify GetManifestPath()
... and don't require a valid build directory when no data from it
is necessary.
---
contrib/testsuite-management/validate_failures.py | 13 +
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/contrib/testsuite-management/validate_failures.py
b/contrib/testsuite-management/validate_failures.py
index 4733dd89dcb..1bd09e0c20c 100755
--- a/contrib/testsuite-management/validate_failures.py
+++ b/contrib/testsuite-management/validate_failures.py
@@ -457,7 +457,7 @@ def CompareResults(manifest, actual):
return actual_vs_manifest, manifest_vs_actual
-def GetManifestPath(srcdir, target, user_provided_must_exist):
+def GetManifestPath(user_provided_must_exist):
"""Return the full path to the manifest file."""
manifest_path = _OPTIONS.manifest
if manifest_path:
@@ -465,6 +465,7 @@ def GetManifestPath(srcdir, target,
user_provided_must_exist):
Error('Manifest does not exist: %s' % manifest_path)
return manifest_path
else:
+(srcdir, target) = GetBuildData()
if not srcdir:
Error('Could not determine the location of GCC\'s source tree. '
'The Makefile does not contain a definition for "srcdir".')
@@ -530,8 +531,7 @@ def PerformComparison(expected, actual,
ignore_missing_failures):
def CheckExpectedResults():
- srcdir, target = GetBuildData()
- manifest_path = GetManifestPath(srcdir, target, True)
+ manifest_path = GetManifestPath(True)
print('Manifest: %s' % manifest_path)
manifest = GetManifest(manifest_path)
sum_files = GetSumFiles(_OPTIONS.results, _OPTIONS.build_dir)
@@ -545,8 +545,7 @@ def CheckExpectedResults():
def ProduceManifest():
- (srcdir, target) = GetBuildData()
- manifest_path = GetManifestPath(srcdir, target, False)
+ manifest_path = GetManifestPath(False)
print('Manifest: %s' % manifest_path)
if os.path.exists(manifest_path) and not _OPTIONS.force:
Error('Manifest file %s already exists.\nUse --force to overwrite.' %
@@ -563,15 +562,13 @@ def ProduceManifest():
def CompareBuilds():
- (srcdir, target) = GetBuildData()
-
sum_files = GetSumFiles(_OPTIONS.results, _OPTIONS.build_dir)
actual = GetResults(sum_files)
clean = ResultSet()
if _OPTIONS.manifest:
-manifest_path = GetManifestPath(srcdir, target, True)
+manifest_path = GetManifestPath(True)
print('Manifest: %s' % manifest_path)
clean = GetManifest(manifest_path)
--
2.34.1
[PATCH 10/12] [contrib] validate_failures.py: Add new option --invert_match
This option is used to detect flaky tests that FAILed in the clean
build (or manifest), but PASSed in the current build (or manifest).
The option inverts output logic similar to what "-v/--invert-match"
does for grep.
---
.../testsuite-management/validate_failures.py | 34 +--
1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/contrib/testsuite-management/validate_failures.py
b/contrib/testsuite-management/validate_failures.py
index 1919935cf53..6eb1acd473f 100755
--- a/contrib/testsuite-management/validate_failures.py
+++ b/contrib/testsuite-management/validate_failures.py
@@ -217,11 +217,17 @@ class ResultSet(set):
Attributes:
current_tool: Name of the current top-level DejaGnu testsuite.
current_exp: Name of the current .exp testsuite file.
+testsuites: A set of (tool, exp) tuples representing encountered
testsuites.
"""
def __init__(self):
super().__init__()
self.ResetToolExp()
+self.testsuites=set()
+
+ def update(self, other):
+super().update(other)
+self.testsuites.update(other.testsuites)
def ResetToolExp(self):
self.current_tool = None
@@ -246,6 +252,10 @@ class ResultSet(set):
outfile.write(_SUMMARY_LINE_FORMAT % 'Results')
+ # Check if testsuite of expected_result is present in current results.
+ # This is used to compare partial test results against a full manifest.
+ def HasTestsuite(self, expected_result):
+return (expected_result.tool, expected_result.exp) in self.testsuites
def GetMakefileValue(makefile_name, value_name):
if os.path.exists(makefile_name):
@@ -391,6 +401,8 @@ def ParseSummary(sum_fname):
result_set.add(result)
elif IsExpLine(line):
result_set.current_exp = _EXP_LINE_REX.match(line).groups()[0]
+ result_set.testsuites.add((result_set.current_tool,
+ result_set.current_exp))
elif IsToolLine(line):
result_set.current_tool = _TOOL_LINE_REX.match(line).groups()[0]
result_set.current_exp = None
@@ -433,7 +445,7 @@ def GetResults(sum_files, build_results = None):
for sum_fname in sum_files:
if _OPTIONS.verbosity >= 3:
print('\t%s' % sum_fname)
-build_results |= ParseSummary(sum_fname)
+build_results.update(ParseSummary(sum_fname))
return build_results
@@ -458,7 +470,11 @@ def CompareResults(manifest, actual):
# Ignore tests marked flaky.
if 'flaky' in expected_result.attrs:
continue
-if expected_result not in actual:
+# We try to support comparing partial results vs full manifest
+# (e.g., manifest has failures for gcc, g++, gfortran, but we ran only
+# g++ testsuite). To achieve this we record encountered testsuites in
+# actual.testsuites set, and then we check it here using HasTestsuite().
+if expected_result not in actual and actual.HasTestsuite(expected_result):
manifest_vs_actual.add(expected_result)
return actual_vs_manifest, manifest_vs_actual
@@ -520,6 +536,13 @@ def GetSumFiles(results, build_dir):
def PerformComparison(expected, actual):
actual_vs_expected, expected_vs_actual = CompareResults(expected, actual)
+ if _OPTIONS.inverse_match:
+# Switch results if inverse comparison is requested.
+# This is useful in detecting flaky tests that FAILed in expected set,
+# but PASSed in actual set.
+actual_vs_expected, expected_vs_actual \
+ = expected_vs_actual, actual_vs_expected
+
tests_ok = True
if len(actual_vs_expected) > 0:
if _OPTIONS.verbosity >= 3:
@@ -613,6 +636,13 @@ def Main(argv):
default=False, help='When used with --produce_manifest, '
'it will overwrite an existing manifest file '
'(default = False)')
+ parser.add_option('--inverse_match', action='store_true',
+dest='inverse_match', default=False,
+help='Inverse result sets in comparison. '
+'Output unexpected passes as unexpected failures and '
+'unexpected failures as unexpected passes. '
+'This is used to catch FAIL->PASS flaky tests. '
+'(default = False)')
parser.add_option('--manifest', action='store', type='string',
dest='manifest', default=None,
help='Name of the manifest file to use (default = '
--
2.34.1
[PATCH 03/12] [contrib] validate_failures.py: Read in manifest when comparing build dirs
This allows comparison of two build directories with a manifest
listing known flaky tests on the side.
---
contrib/testsuite-management/validate_failures.py | 14 +++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/contrib/testsuite-management/validate_failures.py
b/contrib/testsuite-management/validate_failures.py
index 7351ba120b7..4733dd89dcb 100755
--- a/contrib/testsuite-management/validate_failures.py
+++ b/contrib/testsuite-management/validate_failures.py
@@ -420,9 +420,10 @@ def CollectSumFiles(builddir):
return sum_files
-def GetResults(sum_files):
+def GetResults(sum_files, build_results = None):
"""Collect all the test results from the given .sum files."""
- build_results = ResultSet()
+ if build_results == None:
+build_results = ResultSet()
for sum_fname in sum_files:
print('\t%s' % sum_fname)
build_results |= ParseSummary(sum_fname)
@@ -567,8 +568,15 @@ def CompareBuilds():
sum_files = GetSumFiles(_OPTIONS.results, _OPTIONS.build_dir)
actual = GetResults(sum_files)
+ clean = ResultSet()
+
+ if _OPTIONS.manifest:
+manifest_path = GetManifestPath(srcdir, target, True)
+print('Manifest: %s' % manifest_path)
+clean = GetManifest(manifest_path)
+
clean_sum_files = GetSumFiles(_OPTIONS.results, _OPTIONS.clean_build)
- clean = GetResults(clean_sum_files)
+ clean = GetResults(clean_sum_files, clean)
return PerformComparison(clean, actual, _OPTIONS.ignore_missing_failures)
--
2.34.1
[PATCH 02/12] [contrib] validate_failures.py: Support expiry attributes in manifests
---
contrib/testsuite-management/validate_failures.py | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/contrib/testsuite-management/validate_failures.py
b/contrib/testsuite-management/validate_failures.py
index 94ba2e58b51..7351ba120b7 100755
--- a/contrib/testsuite-management/validate_failures.py
+++ b/contrib/testsuite-management/validate_failures.py
@@ -338,7 +338,13 @@ def ParseManifestWorker(result_set, manifest_path):
elif IsInclude(line):
ParseManifestWorker(result_set, GetIncludeFile(line, manifest_path))
elif IsInterestingResult(line):
- result_set.add(result_set.MakeTestResult(line))
+ result = result_set.MakeTestResult(line)
+ if result.HasExpired():
+# Ignore expired manifest entries.
+if _OPTIONS.verbosity >= 4:
+ print('WARNING: Expected failure "%s" has expired.' % line.strip())
+continue
+ result_set.add(result)
elif IsExpLine(orig_line):
result_set.current_exp = _EXP_LINE_REX.match(orig_line).groups()[0]
elif IsToolLine(orig_line):
@@ -369,6 +375,8 @@ def ParseSummary(sum_fname):
result = result_set.MakeTestResult(line, ordinal)
ordinal += 1
if result.HasExpired():
+# ??? What is the use-case for this? How "expiry" annotations are
+# ??? supposed to be added to .sum results?
# Tests that have expired are not added to the set of expected
# results. If they are still present in the set of actual results,
# they will cause an error to be reported.
--
2.34.1
[PATCH 06/12] [contrib] validate_failures.py: Be more stringent in parsing result lines
Before this patch we would identify malformed line
"UNRESOLVEDTest run by tcwg-buildslave on Mon Aug 23 10:17:50 2021"
as an interesting result, only to fail in TestResult:__init__ due
to missing ":" after UNRESOLVED.
This patch makes all places that parse result lines use a single
compiled regex.
---
contrib/testsuite-management/validate_failures.py | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/contrib/testsuite-management/validate_failures.py
b/contrib/testsuite-management/validate_failures.py
index 26ea1d6f53b..f2d7b099d78 100755
--- a/contrib/testsuite-management/validate_failures.py
+++ b/contrib/testsuite-management/validate_failures.py
@@ -60,9 +60,10 @@ import os
import re
import sys
-# Handled test results.
_VALID_TEST_RESULTS = [ 'FAIL', 'UNRESOLVED', 'XPASS', 'ERROR' ]
-_VALID_TEST_RESULTS_REX = re.compile("%s" % "|".join(_VALID_TEST_RESULTS))
+# :
[PATCH 05/12] [contrib] validate_failures.py: Add more verbosity levels
... to control validate_failures.py output
---
.../testsuite-management/validate_failures.py | 82 +++
1 file changed, 46 insertions(+), 36 deletions(-)
diff --git a/contrib/testsuite-management/validate_failures.py
b/contrib/testsuite-management/validate_failures.py
index 1bd09e0c20c..26ea1d6f53b 100755
--- a/contrib/testsuite-management/validate_failures.py
+++ b/contrib/testsuite-management/validate_failures.py
@@ -324,7 +324,7 @@ def GetNegativeResult(line):
def ParseManifestWorker(result_set, manifest_path):
"""Read manifest_path, adding the contents to result_set."""
- if _OPTIONS.verbosity >= 1:
+ if _OPTIONS.verbosity >= 5:
print('Parsing manifest file %s.' % manifest_path)
manifest_file = open(manifest_path, encoding='latin-1', mode='r')
for orig_line in manifest_file:
@@ -380,7 +380,8 @@ def ParseSummary(sum_fname):
# Tests that have expired are not added to the set of expected
# results. If they are still present in the set of actual results,
# they will cause an error to be reported.
-print('WARNING: Expected failure "%s" has expired.' % line.strip())
+if _OPTIONS.verbosity >= 4:
+ print('WARNING: Expected failure "%s" has expired.' % line.strip())
continue
result_set.add(result)
elif IsExpLine(line):
@@ -425,7 +426,8 @@ def GetResults(sum_files, build_results = None):
if build_results == None:
build_results = ResultSet()
for sum_fname in sum_files:
-print('\t%s' % sum_fname)
+if _OPTIONS.verbosity >= 3:
+ print('\t%s' % sum_fname)
build_results |= ParseSummary(sum_fname)
return build_results
@@ -489,42 +491,46 @@ def GetBuildData():
return None, None
srcdir = GetMakefileValue('%s/Makefile' % _OPTIONS.build_dir, 'srcdir =')
target = GetMakefileValue('%s/Makefile' % _OPTIONS.build_dir,
'target_alias=')
- print('Source directory: %s' % srcdir)
- print('Build target: %s' % target)
+ if _OPTIONS.verbosity >= 3:
+print('Source directory: %s' % srcdir)
+print('Build target: %s' % target)
return srcdir, target
-def PrintSummary(msg, summary):
- print('\n\n%s' % msg)
+def PrintSummary(summary):
summary.Print()
def GetSumFiles(results, build_dir):
if not results:
-print('Getting actual results from build directory %s' % build_dir)
+if _OPTIONS.verbosity >= 3:
+ print('Getting actual results from build directory %s' % build_dir)
sum_files = CollectSumFiles(build_dir)
else:
-print('Getting actual results from user-provided results')
+if _OPTIONS.verbosity >= 3:
+ print('Getting actual results from user-provided results')
sum_files = results.split()
return sum_files
-def PerformComparison(expected, actual, ignore_missing_failures):
+def PerformComparison(expected, actual):
actual_vs_expected, expected_vs_actual = CompareResults(expected, actual)
tests_ok = True
if len(actual_vs_expected) > 0:
-PrintSummary('Unexpected results in this build (new failures)',
- actual_vs_expected)
+if _OPTIONS.verbosity >= 3:
+ print('\n\nUnexpected results in this build (new failures)')
+if _OPTIONS.verbosity >= 1:
+ PrintSummary(actual_vs_expected)
tests_ok = False
- if not ignore_missing_failures and len(expected_vs_actual) > 0:
-PrintSummary('Expected results not present in this build (fixed tests)'
- '\n\nNOTE: This is not a failure. It just means that these '
- 'tests were expected\nto fail, but either they worked in '
- 'this configuration or they were not\npresent at all.\n',
- expected_vs_actual)
+ if _OPTIONS.verbosity >= 2 and len(expected_vs_actual) > 0:
+print('\n\nExpected results not present in this build (fixed tests)'
+ '\n\nNOTE: This is not a failure. It just means that these '
+ 'tests were expected\nto fail, but either they worked in '
+ 'this configuration or they were not\npresent at all.\n')
+PrintSummary(expected_vs_actual)
- if tests_ok:
+ if tests_ok and _OPTIONS.verbosity >= 3:
print('\nSUCCESS: No unexpected failures.')
return tests_ok
@@ -532,21 +538,25 @@ def PerformComparison(expected, actual,
ignore_missing_failures):
def CheckExpectedResults():
manifest_path = GetManifestPath(True)
- print('Manifest: %s' % manifest_path)
+ if _OPTIONS.verbosity >= 3:
+print('Manifest: %s' % manifest_path)
manifest = GetManifest(manifest_path)
sum_files = GetSumFiles(_OPTIONS.results, _OPTIONS.build_dir)
actual = GetResults(sum_files)
- if _OPTIONS.verbosity >= 1:
-PrintSummary('Tests expected to fail', manifest)
-PrintSummary('\nActual test results', actual)
+ if _OPTIONS.verbosity >= 5:
+print('\n\nTests expected to fail')
+PrintSummary(manifest)
+print('\n\nActual test results')
+PrintSummary(actual)
- return PerformComparison(ma
[PATCH 07/12] [contrib] validate_failures.py: Use exit code "2" to indicate regression
... in the results. Python exits with code "1" on exceptions and internal errors, which we use to detect failure to parse results. --- contrib/testsuite-management/validate_failures.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/testsuite-management/validate_failures.py b/contrib/testsuite-management/validate_failures.py index f2d7b099d78..c4b9fc377ce 100755 --- a/contrib/testsuite-management/validate_failures.py +++ b/contrib/testsuite-management/validate_failures.py @@ -645,7 +645,7 @@ def Main(argv): if retval: return 0 else: -return 1 +return 2 if __name__ == '__main__': -- 2.34.1
[PATCH 09/12] [contrib] validate_failures.py: Improve error output
From: Thiago Bauermann
- Print message in case of broken sum file error.
- Print error messages to stderr. The script's stdout is, usually,
redirected to a file, and error messages shouldn't go there.
---
contrib/testsuite-management/validate_failures.py | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/contrib/testsuite-management/validate_failures.py
b/contrib/testsuite-management/validate_failures.py
index 6dcdcf5c69b..1919935cf53 100755
--- a/contrib/testsuite-management/validate_failures.py
+++ b/contrib/testsuite-management/validate_failures.py
@@ -136,12 +136,15 @@ class TestResult(object):
self.name,
self.description) =
_VALID_TEST_RESULTS_REX.match(summary_line).groups()
except:
-print('Failed to parse summary line: "%s"' % summary_line)
+print('Failed to parse summary line: "%s"' % summary_line,
+ file=sys.stderr)
raise
self.ordinal = ordinal
if tool == None or exp == None:
# .sum file seem to be broken. There was no "tool" and/or "exp"
# lines preceding this result.
+print(f'.sum file seems to be broken: tool="{tool}", exp="{exp}",
summary_line="{summary_line}"',
+ file=sys.stderr)
raise
self.tool = tool
self.exp = exp
--
2.34.1
[PATCH 11/12] [contrib] validate_failures.py: Add "--expiry_date YYYYMMDD" option
This option sets "today" date to compare expiration entries against.
Setting expiration date into the future allows re-detection of flaky
tests and creating fresh entries for them before the current flaky
entries expire.
---
.../testsuite-management/validate_failures.py | 24 +--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/contrib/testsuite-management/validate_failures.py
b/contrib/testsuite-management/validate_failures.py
index 6eb1acd473f..a77aabe0bdd 100755
--- a/contrib/testsuite-management/validate_failures.py
+++ b/contrib/testsuite-management/validate_failures.py
@@ -206,8 +206,7 @@ class TestResult(object):
# Return True if the expiration date of this result has passed.
expiration_date = self.ExpirationDate()
if expiration_date:
- now = datetime.date.today()
- return now > expiration_date
+ return _OPTIONS.expiry_today_date > expiration_date
class ResultSet(set):
@@ -636,6 +635,11 @@ def Main(argv):
default=False, help='When used with --produce_manifest, '
'it will overwrite an existing manifest file '
'(default = False)')
+ parser.add_option('--expiry_date', action='store',
+dest='expiry_today_date', default=None,
+help='Use provided date MMDD to decide whether '
+'manifest entries with expiry settings have expired '
+'or not. (default = Use today date)')
parser.add_option('--inverse_match', action='store_true',
dest='inverse_match', default=False,
help='Inverse result sets in comparison. '
@@ -670,6 +674,22 @@ def Main(argv):
global _OPTIONS
(_OPTIONS, _) = parser.parse_args(argv[1:])
+ # Set "today" date to compare expiration entries against.
+ # Setting expiration date into the future allows re-detection of flaky
+ # tests and creating fresh entries for them before the current flaky entries
+ # expire.
+ if _OPTIONS.expiry_today_date:
+today_date = re.search(r'(\d\d\d\d)(\d\d)(\d\d)',
+ _OPTIONS.expiry_today_date)
+if not today_date:
+Error('Invalid --expiry_today_date format "%s". Must be of the form '
+ '"expire=MMDD"' % _OPTIONS.expiry_today_date)
+_OPTIONS.expiry_today_date=datetime.date(int(today_date.group(1)),
+ int(today_date.group(2)),
+ int(today_date.group(3)))
+ else:
+_OPTIONS.expiry_today_date = datetime.date.today()
+
if _OPTIONS.produce_manifest:
retval = ProduceManifest()
elif _OPTIONS.clean_build:
--
2.34.1
[PATCH 08/12] [contrib] validate_failures.py: Support "$tool:" prefix in exp names
From: Christophe Lyon
This makes it easier to extract the $tool:$exp pair when iterating
over failures/flaky tests, which, in turn, simplifies re-running
testsuite parts that have unexpected failures or passes.
---
contrib/testsuite-management/validate_failures.py | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/contrib/testsuite-management/validate_failures.py
b/contrib/testsuite-management/validate_failures.py
index c4b9fc377ce..6dcdcf5c69b 100755
--- a/contrib/testsuite-management/validate_failures.py
+++ b/contrib/testsuite-management/validate_failures.py
@@ -67,12 +67,14 @@ _VALID_TEST_RESULTS_REX = re.compile('(%s):\s*(\S+)\s*(.*)'
# Formats of .sum file sections
_TOOL_LINE_FORMAT = '\t\t=== %s tests ===\n'
-_EXP_LINE_FORMAT = '\nRunning %s ...\n'
+_EXP_LINE_FORMAT = '\nRunning %s:%s ...\n'
_SUMMARY_LINE_FORMAT = '\n\t\t=== %s Summary ===\n'
# ... and their compiled regexs.
_TOOL_LINE_REX = re.compile('^\t\t=== (.*) tests ===\n')
-_EXP_LINE_REX = re.compile('^Running (.*\.exp) \.\.\.\n')
+# Match .exp file name, optionally prefixed by a "tool:" name and a
+# path ending with "testsuite/"
+_EXP_LINE_REX = re.compile('^Running (?:.*:)?(.*) \.\.\.\n')
_SUMMARY_LINE_REX = re.compile('^\t\t=== (.*) Summary ===\n')
# Subdirectory of srcdir in which to find the manifest file.
@@ -236,7 +238,7 @@ class ResultSet(set):
outfile.write(_TOOL_LINE_FORMAT % current_tool)
if current_exp != result.exp:
current_exp = result.exp
-outfile.write(_EXP_LINE_FORMAT % current_exp)
+outfile.write(_EXP_LINE_FORMAT % (current_tool, current_exp))
outfile.write('%s\n' % result)
outfile.write(_SUMMARY_LINE_FORMAT % 'Results')
--
2.34.1
[PATCH 12/12] [contrib] validate_failures.py: Ignore stray filesystem paths in results
This patch simplifies comparison of results that have filesystem
paths. E.g., (assuming different values of ):
Running
/home/user/gcc-N/gcc/testsuite/gcc.target/aarch64/sve/acle/aarch64-sve-acle-asm.exp
...
ERROR: tcl error sourcing
/home/user/gcc-N/gcc/testsuite/gcc.target/aarch64/sve/acle/aarch64-sve-acle-asm.exp.
We add "--srcpath ", option, and set it by default to
"[^ ]+/testsuite/", which works well for all components of the GNU
Toolchain. We then remove substrings matching from paths of
.exp files and from occasional "ERROR:" results.
---
contrib/testsuite-management/validate_failures.py | 15 +++
1 file changed, 15 insertions(+)
diff --git a/contrib/testsuite-management/validate_failures.py
b/contrib/testsuite-management/validate_failures.py
index a77aabe0bdd..4dfd9cda4e2 100755
--- a/contrib/testsuite-management/validate_failures.py
+++ b/contrib/testsuite-management/validate_failures.py
@@ -135,6 +135,9 @@ class TestResult(object):
(self.state,
self.name,
self.description) =
_VALID_TEST_RESULTS_REX.match(summary_line).groups()
+if _OPTIONS.srcpath_regex and _OPTIONS.srcpath_regex != '':
+ self.description = re.sub(_OPTIONS.srcpath_regex, '',
+self.description)
except:
print('Failed to parse summary line: "%s"' % summary_line,
file=sys.stderr)
@@ -361,6 +364,9 @@ def ParseManifestWorker(result_set, manifest_path):
result_set.add(result)
elif IsExpLine(orig_line):
result_set.current_exp = _EXP_LINE_REX.match(orig_line).groups()[0]
+ if _OPTIONS.srcpath_regex and _OPTIONS.srcpath_regex != '':
+result_set.current_exp = re.sub(_OPTIONS.srcpath_regex, '',
+result_set.current_exp)
elif IsToolLine(orig_line):
result_set.current_tool = _TOOL_LINE_REX.match(orig_line).groups()[0]
elif IsSummaryLine(orig_line):
@@ -400,6 +406,9 @@ def ParseSummary(sum_fname):
result_set.add(result)
elif IsExpLine(line):
result_set.current_exp = _EXP_LINE_REX.match(line).groups()[0]
+ if _OPTIONS.srcpath_regex and _OPTIONS.srcpath_regex != '':
+result_set.current_exp = re.sub(_OPTIONS.srcpath_regex, '',
+result_set.current_exp)
result_set.testsuites.add((result_set.current_tool,
result_set.current_exp))
elif IsToolLine(line):
@@ -640,6 +649,12 @@ def Main(argv):
help='Use provided date MMDD to decide whether '
'manifest entries with expiry settings have expired '
'or not. (default = Use today date)')
+ parser.add_option('--srcpath', action='store', type='string',
+dest='srcpath_regex', default='[^ ]+/testsuite/',
+help='Remove provided path (can be a regex) from '
+'the result entries. This is useful to remove '
+'occasional filesystem path from the results. '
+'(default = "[^ ]+/testsuite/")')
parser.add_option('--inverse_match', action='store_true',
dest='inverse_match', default=False,
help='Inverse result sets in comparison. '
--
2.34.1
Re: [pushed] analyzer: implement various atomic builtins [PR109015]
Hi David,
The new test ICEs the compiler on aarch64-linux-gnu [1]. Would you please
investigate?
Running gcc:gcc.dg/analyzer/analyzer.exp ...
FAIL: gcc.dg/analyzer/atomic-builtins-qemu-sockets.c (internal compiler error:
in validate, at analyzer/store.cc:1329)
FAIL: gcc.dg/analyzer/atomic-builtins-qemu-sockets.c (test for excess errors)
This is a simple native build on aarch64-linux-gnu. Please let me know if you
need any help in reproducing this.
[1]
https://ci.linaro.org/job/tcwg_gcc_check--master-aarch64-build/82/artifact/artifacts/artifacts.precommit/06-check_regression/results.compare/*view*/
Thanks!
--
Maxim Kuvyrkov
https://www.linaro.org
> On Jun 2, 2023, at 17:32, David Malcolm via Gcc-patches
> wrote:
>
> This patch implements many of the __atomic_* builtins from
> sync-builtins.def as known_function subclasses within the analyzer.
>
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
> Pushed to trunk as r14-1497-gef768035ae8090.
>
> gcc/analyzer/ChangeLog:
> PR analyzer/109015
> * kf.cc (class kf_atomic_exchange): New.
> (class kf_atomic_exchange_n): New.
> (class kf_atomic_fetch_op): New.
> (class kf_atomic_op_fetch): New.
> (class kf_atomic_load): New.
> (class kf_atomic_load_n): New.
> (class kf_atomic_store_n): New.
> (register_atomic_builtins): New function.
> (register_known_functions): Call register_atomic_builtins.
>
> gcc/testsuite/ChangeLog:
> PR analyzer/109015
> * gcc.dg/analyzer/atomic-builtins-1.c: New test.
> * gcc.dg/analyzer/atomic-builtins-haproxy-proxy.c: New test.
> * gcc.dg/analyzer/atomic-builtins-qemu-sockets.c: New test.
> * gcc.dg/analyzer/atomic-types-1.c: New test.
> ---
> gcc/analyzer/kf.cc| 355
> .../gcc.dg/analyzer/atomic-builtins-1.c | 544 ++
> .../analyzer/atomic-builtins-haproxy-proxy.c | 55 ++
> .../analyzer/atomic-builtins-qemu-sockets.c | 18 +
> .../gcc.dg/analyzer/atomic-types-1.c | 11 +
> 5 files changed, 983 insertions(+)
> create mode 100644 gcc/testsuite/gcc.dg/analyzer/atomic-builtins-1.c
> create mode 100644
> gcc/testsuite/gcc.dg/analyzer/atomic-builtins-haproxy-proxy.c
> create mode 100644
> gcc/testsuite/gcc.dg/analyzer/atomic-builtins-qemu-sockets.c
> create mode 100644 gcc/testsuite/gcc.dg/analyzer/atomic-types-1.c
>
> diff --git a/gcc/analyzer/kf.cc b/gcc/analyzer/kf.cc
> index 93c46630f36..104499e 100644
> --- a/gcc/analyzer/kf.cc
> +++ b/gcc/analyzer/kf.cc
> @@ -69,6 +69,235 @@ kf_alloca::impl_call_pre (const call_details &cd) const
> cd.maybe_set_lhs (ptr_sval);
> }
>
> +/* Handler for:
> + void __atomic_exchange (type *ptr, type *val, type *ret, int memorder).
> */
> +
> +class kf_atomic_exchange : public internal_known_function
> +{
> +public:
> + /* This is effectively:
> + *RET = *PTR;
> + *PTR = *VAL;
> + */
> + void impl_call_pre (const call_details &cd) const final override
> + {
> +const svalue *ptr_ptr_sval = cd.get_arg_svalue (0);
> +tree ptr_ptr_tree = cd.get_arg_tree (0);
> +const svalue *val_ptr_sval = cd.get_arg_svalue (1);
> +tree val_ptr_tree = cd.get_arg_tree (1);
> +const svalue *ret_ptr_sval = cd.get_arg_svalue (2);
> +tree ret_ptr_tree = cd.get_arg_tree (2);
> +/* Ignore the memorder param. */
> +
> +region_model *model = cd.get_model ();
> +region_model_context *ctxt = cd.get_ctxt ();
> +
> +const region *val_region
> + = model->deref_rvalue (val_ptr_sval, val_ptr_tree, ctxt);
> +const svalue *star_val_sval = model->get_store_value (val_region, ctxt);
> +const region *ptr_region
> + = model->deref_rvalue (ptr_ptr_sval, ptr_ptr_tree, ctxt);
> +const svalue *star_ptr_sval = model->get_store_value (ptr_region, ctxt);
> +const region *ret_region
> + = model->deref_rvalue (ret_ptr_sval, ret_ptr_tree, ctxt);
> +model->set_value (ptr_region, star_val_sval, ctxt);
> +model->set_value (ret_region, star_ptr_sval, ctxt);
> + }
> +};
> +
> +/* Handler for:
> + __atomic_exchange_n (type *ptr, type val, int memorder). */
> +
> +class kf_atomic_exchange_n : public internal_known_function
> +{
> +public:
> + /* This is effectively:
> + RET = *PTR;
> + *PTR = VAL;
> + return RET;
> + */
> + void impl_call_pre (const call_details &cd) const final override
> + {
> +const svalue *ptr_sval = cd.get_arg_svalue (0);
> +tree ptr_tree = cd.get_arg_tree (0);
> +const svalue *set_sval = cd.get_arg_svalue (1);
> +/* Ignore the memorder param. */
> +
> +region_model *model = cd.get_model ();
> +region_model_context *ctxt = cd.get_ctxt ();
> +
> +const region *dst_region = model->deref_rvalue (ptr_sval, ptr_tree,
> ctxt);
> +const svalue *ret_sval = model->get_store_value (dst_region, ctxt);
> +model->set_value (dst_region, set_sval, ctxt);
> +cd.maybe_set_lhs (ret_sval);
> + }
> +};
> +
> +/* Handler for:
> + type __atomic_fetch_add (type *ptr, type val, int memo
Re: [pushed] analyzer: implement various atomic builtins [PR109015]
Hi David,
Hm, I'm seeing this failure only in pre-commit testing, but I don't see it in
our post-commit testing of gcc:master.
Does this patch rely on your other patch committed just before this one?
--
Maxim Kuvyrkov
https://www.linaro.org
> On Jun 3, 2023, at 09:23, Maxim Kuvyrkov wrote:
>
> Hi David,
>
> The new test ICEs the compiler on aarch64-linux-gnu [1]. Would you please
> investigate?
>
> Running gcc:gcc.dg/analyzer/analyzer.exp ...
> FAIL: gcc.dg/analyzer/atomic-builtins-qemu-sockets.c (internal compiler
> error: in validate, at analyzer/store.cc:1329)
> FAIL: gcc.dg/analyzer/atomic-builtins-qemu-sockets.c (test for excess errors)
>
> This is a simple native build on aarch64-linux-gnu. Please let me know if
> you need any help in reproducing this.
>
> [1]
> https://ci.linaro.org/job/tcwg_gcc_check--master-aarch64-build/82/artifact/artifacts/artifacts.precommit/06-check_regression/results.compare/*view*/
>
> Thanks!
>
> --
> Maxim Kuvyrkov
> https://www.linaro.org
>
>
>
>
>> On Jun 2, 2023, at 17:32, David Malcolm via Gcc-patches
>> wrote:
>>
>> This patch implements many of the __atomic_* builtins from
>> sync-builtins.def as known_function subclasses within the analyzer.
>>
>> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
>> Pushed to trunk as r14-1497-gef768035ae8090.
>>
>> gcc/analyzer/ChangeLog:
>> PR analyzer/109015
>> * kf.cc (class kf_atomic_exchange): New.
>> (class kf_atomic_exchange_n): New.
>> (class kf_atomic_fetch_op): New.
>> (class kf_atomic_op_fetch): New.
>> (class kf_atomic_load): New.
>> (class kf_atomic_load_n): New.
>> (class kf_atomic_store_n): New.
>> (register_atomic_builtins): New function.
>> (register_known_functions): Call register_atomic_builtins.
>>
>> gcc/testsuite/ChangeLog:
>> PR analyzer/109015
>> * gcc.dg/analyzer/atomic-builtins-1.c: New test.
>> * gcc.dg/analyzer/atomic-builtins-haproxy-proxy.c: New test.
>> * gcc.dg/analyzer/atomic-builtins-qemu-sockets.c: New test.
>> * gcc.dg/analyzer/atomic-types-1.c: New test.
>> ---
>> gcc/analyzer/kf.cc| 355
>> .../gcc.dg/analyzer/atomic-builtins-1.c | 544 ++
>> .../analyzer/atomic-builtins-haproxy-proxy.c | 55 ++
>> .../analyzer/atomic-builtins-qemu-sockets.c | 18 +
>> .../gcc.dg/analyzer/atomic-types-1.c | 11 +
>> 5 files changed, 983 insertions(+)
>> create mode 100644 gcc/testsuite/gcc.dg/analyzer/atomic-builtins-1.c
>> create mode 100644
>> gcc/testsuite/gcc.dg/analyzer/atomic-builtins-haproxy-proxy.c
>> create mode 100644
>> gcc/testsuite/gcc.dg/analyzer/atomic-builtins-qemu-sockets.c
>> create mode 100644 gcc/testsuite/gcc.dg/analyzer/atomic-types-1.c
>>
>> diff --git a/gcc/analyzer/kf.cc b/gcc/analyzer/kf.cc
>> index 93c46630f36..104499e 100644
>> --- a/gcc/analyzer/kf.cc
>> +++ b/gcc/analyzer/kf.cc
>> @@ -69,6 +69,235 @@ kf_alloca::impl_call_pre (const call_details &cd) const
>> cd.maybe_set_lhs (ptr_sval);
>> }
>>
>> +/* Handler for:
>> + void __atomic_exchange (type *ptr, type *val, type *ret, int memorder).
>> */
>> +
>> +class kf_atomic_exchange : public internal_known_function
>> +{
>> +public:
>> + /* This is effectively:
>> + *RET = *PTR;
>> + *PTR = *VAL;
>> + */
>> + void impl_call_pre (const call_details &cd) const final override
>> + {
>> +const svalue *ptr_ptr_sval = cd.get_arg_svalue (0);
>> +tree ptr_ptr_tree = cd.get_arg_tree (0);
>> +const svalue *val_ptr_sval = cd.get_arg_svalue (1);
>> +tree val_ptr_tree = cd.get_arg_tree (1);
>> +const svalue *ret_ptr_sval = cd.get_arg_svalue (2);
>> +tree ret_ptr_tree = cd.get_arg_tree (2);
>> +/* Ignore the memorder param. */
>> +
>> +region_model *model = cd.get_model ();
>> +region_model_context *ctxt = cd.get_ctxt ();
>> +
>> +const region *val_region
>> + = model->deref_rvalue (val_ptr_sval, val_ptr_tree, ctxt);
>> +const svalue *star_val_sval = model->get_store_value (val_region, ctxt);
>> +const region *ptr_region
>> + = model->deref_rvalue (ptr_ptr_sval, ptr_ptr_tree, ctxt);
>> +const svalue *star_ptr_sval = model->get_store_value (ptr_region, ctxt);
>> +const region *ret_region
>> + = model->deref_rvalue (ret_ptr_sval, ret_ptr_tree, ctxt);
>> +model->set_value (ptr_region, star_val_sval, ctxt);
>> +model->set_value (ret_region, star_ptr_sval, ctxt);
>> + }
>> +};
>> +
>> +/* Handler for:
>> + __atomic_exchange_n (type *ptr, type val, int memorder). */
>> +
>> +class kf_atomic_exchange_n : public internal_known_function
>> +{
>> +public:
>> + /* This is effectively:
>> + RET = *PTR;
>> + *PTR = VAL;
>> + return RET;
>> + */
>> + void impl_call_pre (const call_details &cd) const final override
>> + {
>> +const svalue *ptr_sval = cd.get_arg_svalue (0);
>> +tree ptr_tree = cd.get_arg_tree (0);
>> +const svalue *set_sval = cd.get_arg_svalue (1);
>> +/* Ignore the memorder param.
Re: [PATCH 01/12] [contrib] validate_failures.py: Avoid testsuite aliasing
> On Jun 3, 2023, at 19:17, Jeff Law wrote: > > On 6/2/23 09:20, Maxim Kuvyrkov via Gcc-patches wrote: >> This patch adds tracking of current testsuite "tool" and "exp" >> to the processing of .sum files. This avoids aliasing between >> tests from different testsuites with same name+description. >> E.g., this is necessary for testsuite/c-c++-common, which is ran >> for both gcc and g++ "tools". >> This patch changes manifest format from ... >> >> FAIL: gcc_test >> FAIL: g++_test >> >> ... to ... >> >> === gcc tests === >> Running gcc/foo.exp ... >> FAIL: gcc_test >> === gcc Summary == >> === g++ tests === >> Running g++/bar.exp ... >> FAIL: g++_test >> === g++ Summary == >> . >> The new format uses same formatting as DejaGnu's .sum files >> to specify which "tool" and "exp" the test belongs to. > I think the series is fine. You're not likely to hear from Diego or Doug I > suspect, I don't think either are involved in GNU stuff anymore. > Thanks, Jeff. I'll wait for a couple of days and will merge if there are no new comments. Kind regards, -- Maxim Kuvyrkov https://www.linaro.org
Re: [r14-1579 Regression] FAIL: gfortran.dg/gomp/target-update-1.f90 -O scan-tree-dump gimple "#pragma omp target update to\\(c \\[len: [0-9]+\\]\\) to\\(present:a \\[len: [0-9]+\\]\\) to\\(e \\[len:
Hi Tobias,
We are also seeing this failure on arm-linux-gnueabihf.
It seems the problem is different order of variables in output:
- c a e d b
versus expected
- c a e b d
This is tree-dump output on arm-linux-gnueabihf:
#pragma omp target update to(c [len: 4]) to(present:a [len: 4000]) to(e
[len: 4]) from(d [len: 4]) from(present:b [len: 4000])
Let me know if you need any help in troubleshooting this.
--
Maxim Kuvyrkov
https://www.linaro.org
> On Jun 7, 2023, at 02:50, haochen.jiang via Gcc-patches
> wrote:
>
> On Linux/x86_64,
>
> 4ede915d5dde935a16df2c6640aee5ab22348d30 is the first bad commit
> commit 4ede915d5dde935a16df2c6640aee5ab22348d30
> Author: Tobias Burnus
> Date: Tue Jun 6 16:47:16 2023 +0200
>
>openmp: Add support for the 'present' modifier
>
> caused
>
> FAIL: gfortran.dg/gomp/target-update-1.f90 -O scan-tree-dump gimple
> "#pragma omp target update to\\(c \\[len: [0-9]+\\]\\) to\\(present:a \\[len:
> [0-9]+\\]\\) to\\(e \\[len: [0-9]+\\]\\) from\\(present:b \\[len:
> [0-9]+\\]\\) from\\(d \\[len: [0-9]+\\]\\)"
>
> with GCC configured with
>
> ../../gcc/configure
> --prefix=/export/users/haochenj/src/gcc-bisect/master/master/r14-1579/usr
> --enable-clocale=gnu --with-system-zlib --with-demangler-in-ld
> --with-fpmath=sse --enable-languages=c,c++,fortran --enable-cet --without-isl
> --enable-libmpx x86_64-linux --disable-bootstrap
>
> To reproduce:
>
> $ cd {build_dir}/gcc && make check
> RUNTESTFLAGS="gomp.exp=gfortran.dg/gomp/target-update-1.f90
> --target_board='unix{-m32}'"
> $ cd {build_dir}/gcc && make check
> RUNTESTFLAGS="gomp.exp=gfortran.dg/gomp/target-update-1.f90
> --target_board='unix{-m32\ -march=cascadelake}'"
> $ cd {build_dir}/gcc && make check
> RUNTESTFLAGS="gomp.exp=gfortran.dg/gomp/target-update-1.f90
> --target_board='unix{-m64}'"
> $ cd {build_dir}/gcc && make check
> RUNTESTFLAGS="gomp.exp=gfortran.dg/gomp/target-update-1.f90
> --target_board='unix{-m64\ -march=cascadelake}'"
>
> (Please do not reply to this email, for question about this report, contact
> me at haochen dot jiang at intel.com)
Re: [committed] libstdc++: Fix code size regressions in std::vector [PR110060]
Hi Jonathan,
Interestingly, this increases code-size of -O3 code on aarch64-linux-gnu on
SPEC CPU2017's 641.leela_s benchmark [1].
In particular, FastBoard::get_nearby_enemies() grew from 1444 to 2212 bytes.
This seems like a corner-case; the rest of SPEC CPU2017 is, mostly, neutral to
this patch. Is this something you may be interested in investigating? I'll be
happy to assist.
Looking at assembly, one of the differences I see is that the "after" version
has calls to realloc_insert(), while "before" version seems to have them
inlined [2].
[1]
https://git.linaro.org/toolchain/ci/interesting-commits.git/tree/gcc/sha1/b7b255e77a271974479c34d1db3daafc04b920bc/tcwg_bmk-code_size-cpu2017fast/status.txt
[2] 641.leela_s is non-GPL/non-BSD benchmark, and I'm not sure if I can post
its compiled and/or preprocessed code publicly. I assume RedHat has SPEC
CPU2017 license, and I can post details to you privately.
Kind regards,
--
Maxim Kuvyrkov
https://www.linaro.org
> On Jun 1, 2023, at 19:09, Jonathan Wakely via Gcc-patches
> wrote:
>
> Tested powerpc64le-linux. Pusshed to trunk.
>
> -- >8 --
>
> My r14-1452-gfb409a15d9babc change to add optimization hints to
> std::vector causes regressions because it makes std::vector::size() and
> std::vector::capacity() too big to inline. That's the opposite of what
> I wanted, so revert the changes to those functions.
>
> To achieve the original aim of optimizing vec.assign(vec.size(), x) we
> can add a local optimization hint to _M_fill_assign, so that it doesn't
> affect all other uses of size() and capacity().
>
> Additionally, add the same hint to the _M_assign_aux overload for
> forward iterators and add that to the testcase.
>
> It would be nice to similarly optimize:
> if (vec1.size() == vec2.size()) vec1 = vec2;
> but adding hints to operator=(const vector&) doesn't help. Presumably
> the relationships between the two sizes and two capacities are too
> complex to track effectively.
>
> libstdc++-v3/ChangeLog:
>
> PR libstdc++/110060
> * include/bits/stl_vector.h (_Vector_base::_M_invariant):
> Remove.
> (vector::size, vector::capacity): Remove calls to _M_invariant.
> * include/bits/vector.tcc (vector::_M_fill_assign): Add
> optimization hint to reallocating path.
> (vector::_M_assign_aux(FwdIter, FwdIter, forward_iterator_tag)):
> Likewise.
> * testsuite/23_containers/vector/capacity/invariant.cc: Moved
> to...
> * testsuite/23_containers/vector/modifiers/assign/no_realloc.cc:
> ...here. Check assign(FwdIter, FwdIter) too.
> * testsuite/23_containers/vector/types/1.cc: Revert addition
> of -Wno-stringop-overread option.
> ---
> libstdc++-v3/include/bits/stl_vector.h| 23 +--
> libstdc++-v3/include/bits/vector.tcc | 17 ++
> .../assign/no_realloc.cc} | 6 +
> .../testsuite/23_containers/vector/types/1.cc | 2 +-
> 4 files changed, 20 insertions(+), 28 deletions(-)
> rename libstdc++-v3/testsuite/23_containers/vector/{capacity/invariant.cc =>
> modifiers/assign/no_realloc.cc} (70%)
>
> diff --git a/libstdc++-v3/include/bits/stl_vector.h
> b/libstdc++-v3/include/bits/stl_vector.h
> index e593be443bc..70ced3d101f 100644
> --- a/libstdc++-v3/include/bits/stl_vector.h
> +++ b/libstdc++-v3/include/bits/stl_vector.h
> @@ -389,23 +389,6 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
>
> protected:
>
> - __attribute__((__always_inline__))
> - _GLIBCXX20_CONSTEXPR void
> - _M_invariant() const
> - {
> -#if __OPTIMIZE__
> - if (this->_M_impl._M_finish < this->_M_impl._M_start)
> - __builtin_unreachable();
> - if (this->_M_impl._M_finish > this->_M_impl._M_end_of_storage)
> - __builtin_unreachable();
> -
> - size_t __sz = this->_M_impl._M_finish - this->_M_impl._M_start;
> - size_t __cap = this->_M_impl._M_end_of_storage - this->_M_impl._M_start;
> - if (__sz > __cap)
> - __builtin_unreachable();
> -#endif
> - }
> -
> _GLIBCXX20_CONSTEXPR
> void
> _M_create_storage(size_t __n)
> @@ -1005,10 +988,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
> _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
> size_type
> size() const _GLIBCXX_NOEXCEPT
> - {
> - _Base::_M_invariant();
> - return size_type(this->_M_impl._M_finish - this->_M_impl._M_start);
> - }
> + { return size_type(this->_M_impl._M_finish - this->_M_impl._M_start); }
>
> /** Returns the size() of the largest possible %vector. */
> _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
> @@ -1095,7 +1075,6 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
> size_type
> capacity() const _GLIBCXX_NOEXCEPT
> {
> - _Base::_M_invariant();
> return size_type(this->_M_impl._M_end_of_storage
> - this->_M_impl._M_start);
> }
> diff --git a/libstdc++-v3/include/bits/vector.tcc
> b/libstdc++-v3/include/bits/vector.tcc
> index d6fdea2dd01..acd11e2dc68 100644
> --- a/libstdc++-v3/include/bits/vector.tcc
> +++ b/libstdc++-v3/include/bits/vector.tcc
> @@ -270,15 +270,
Re: [PATCH] Handle FMA friendly in reassoc pass
> On May 25, 2023, at 03:30, Cui, Lili via Gcc-patches > wrote: > > From: Lili Cui > > Make some changes in reassoc pass to make it more friendly to fma pass later. > Using FMA instead of mult + add reduces register pressure and insruction > retired. > > There are mainly two changes > 1. Put no-mult ops and mult ops alternately at the end of the queue, which is > conducive to generating more fma and reducing the loss of FMA when breaking > the chain. > 2. Rewrite the rewrite_expr_tree_parallel function to try to build parallel > chains according to the given correlation width, keeping the FMA chance as > much as possible. > > With the patch applied > > On ICX: > 507.cactuBSSN_r: Improved by 1.7% for multi-copy . > 503.bwaves_r : Improved by 0.60% for single copy . > 507.cactuBSSN_r: Improved by 1.10% for single copy . > 519.lbm_r : Improved by 2.21% for single copy . > no measurable changes for other benchmarks. > > On aarch64 > 507.cactuBSSN_r: Improved by 1.7% for multi-copy. > 503.bwaves_r : Improved by 6.00% for single-copy. > no measurable changes for other benchmarks. Hi Cui, I'm seeing a 4% slowdown on 436.cactusADM from SPEC CPU2006 on aarch64-linux-gnu (Cortex-A57) when compiling with "-O2 -flto". All other benchmarks seem neutral to this patch, and I didn't observe the slow down with plain -O2 no-LTO or with -O3. Is this something interesting to investigate? I'll be happy to assist. Kind regards, -- Maxim Kuvyrkov https://www.linaro.org
Re: [PATCH] analyzer: Standalone OOB-warning [PR109437, PR109439]
> On Jun 6, 2023, at 15:48, Benjamin Priour via Gcc-patches > wrote: > > From: Benjamin Priour > > This patch enchances -Wanalyzer-out-of-bounds that is no longer paired > with a -Wanalyzer-use-of-uninitialized-value on out-of-bounds-read. > > This also fixes PR analyzer/109437. > Before there could always be at most one OOB-read warning per frame because > -Wanalyzer-use-of-uninitialized-value always terminates the analysis > path. > > PR analyzer/109439 > > gcc/analyzer/ChangeLog: > > * bounds-checking.cc (region_model::check_symbolic_bounds): > Returns whether the BASE_REG region access was OOB. > (region_model::check_region_bounds): Likewise. > * region-model.cc (region_model::get_store_value): Creates an > unknown svalue on OOB-read access to REG. > (region_model::check_region_access): Returns whether an > unknown svalue needs be created. > (region_model::check_region_for_read): Passes > check_region_access return value. > * region-model.h: Update prior function definitions. > > gcc/testsuite/ChangeLog: > > * gcc.dg/analyzer/out-of-bounds-2.c: Cleaned test for > uninitialized-value warning. > * gcc.dg/analyzer/out-of-bounds-5.c: Likewise. > * gcc.dg/analyzer/pr101962.c: Likewise. > * gcc.dg/analyzer/realloc-5.c: Likewise. > * gcc.dg/analyzer/pr109439.c: New test. > --- Hi Benjamin, This patch makes two tests fail on arm-linux-gnueabihf. Probably, they need to be updated as well. Would you please investigate? Let me know if it doesn't easily reproduce for you, and I'll help. === g++ tests === Running g++:g++.dg/analyzer/analyzer.exp ... FAIL: g++.dg/analyzer/pr100244.C -std=c++14 (test for warnings, line 17) FAIL: g++.dg/analyzer/pr100244.C -std=c++17 (test for warnings, line 17) FAIL: g++.dg/analyzer/pr100244.C -std=c++20 (test for warnings, line 17) === gcc tests === Running gcc:gcc.dg/analyzer/analyzer.exp ... FAIL: gcc.dg/analyzer/pr101962.c (test for warnings, line 19) Thanks, -- Maxim Kuvyrkov https://www.linaro.org > gcc/analyzer/bounds-checking.cc | 30 +-- > gcc/analyzer/region-model.cc | 22 +- > gcc/analyzer/region-model.h | 8 ++--- > .../gcc.dg/analyzer/out-of-bounds-2.c | 1 - > .../gcc.dg/analyzer/out-of-bounds-5.c | 2 -- > gcc/testsuite/gcc.dg/analyzer/pr101962.c | 1 - > gcc/testsuite/gcc.dg/analyzer/pr109439.c | 12 > gcc/testsuite/gcc.dg/analyzer/realloc-5.c | 1 - > 8 files changed, 51 insertions(+), 26 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/analyzer/pr109439.c > > diff --git a/gcc/analyzer/bounds-checking.cc b/gcc/analyzer/bounds-checking.cc > index 3bf542a8eba..479b8e4b88d 100644 > --- a/gcc/analyzer/bounds-checking.cc > +++ b/gcc/analyzer/bounds-checking.cc > @@ -767,9 +767,11 @@ public: > } > }; > > -/* Check whether an access is past the end of the BASE_REG. */ > +/* Check whether an access is past the end of the BASE_REG. > + Return TRUE if the access was valid, FALSE otherwise. > +*/ > > -void > +bool > region_model::check_symbolic_bounds (const region *base_reg, > const svalue *sym_byte_offset, > const svalue *num_bytes_sval, > @@ -800,6 +802,7 @@ region_model::check_symbolic_bounds (const region > *base_reg, > offset_tree, > num_bytes_tree, > capacity_tree)); > +return false; > break; > case DIR_WRITE: > ctxt->warn (make_unique (base_reg, > @@ -807,9 +810,11 @@ region_model::check_symbolic_bounds (const region > *base_reg, > offset_tree, > num_bytes_tree, > capacity_tree)); > +return false; > break; > } > } > + return true; > } > > static tree > @@ -822,9 +827,11 @@ maybe_get_integer_cst_tree (const svalue *sval) > return NULL_TREE; > } > > -/* May complain when the access on REG is out-of-bounds. */ > +/* May complain when the access on REG is out-of-bounds. > + Return TRUE if the access was valid, FALSE otherwise. > +*/ > > -void > +bool > region_model::check_region_bounds (const region *reg, > enum access_direction dir, > region_model_context *ctxt) const > @@ -839,14 +846,14 @@ region_model::check_region_bounds (const region *reg, > (e.g. because the analyzer did not see previous offsets on the latter, > it might think that a negative access is before the buffer). */ > if (base_reg->symbolic_p ()) > -return; > +return true; > > /* Find out how many bytes were accessed. */ > const svalue *num_bytes_sval = reg->get_byte_size_sval (m_mgr); > tree num_bytes_tree = maybe_get_integer_cst_tree (num_bytes_sval); > /* Bail out if 0 bytes are accessed. */ > if (num_bytes_tree && zerop (num_bytes_tree)) > -return; > +return true; > > /* Get the capacity of the buffer. */ > const svalue *capacity = get_capacity (base_reg); > @@ -877,13 +884,13 @@ region_model::check_region_bounds (const region *reg, > } > else > byte_offset_sval = reg_offset.get_symbolic_byte_offset (); > -
Re: [PATCH v6 0/4] P1689R5 support
> On Jun 7, 2023, at 00:50, Ben Boeckel via Gcc-patches > wrote: > > Hi, > > This patch series adds initial support for ISO C++'s [P1689R5][], a > format for describing C++ module requirements and provisions based on > the source code. This is required because compiling C++ with modules is > not embarrassingly parallel and need to be ordered to ensure that > `import some_module;` can be satisfied in time by making sure that any > TU with `export import some_module;` is compiled first. Hi Ben, This patch series causes ICEs on arm-linux-gnueabihf. Would you please investigate? Please let me know if you need any in reproducing these. === g++ tests === Running g++:g++.dg/modules/modules.exp ... FAIL: g++.dg/modules/ben-1_a.C -std=c++17 (internal compiler error: Segmentation fault) FAIL: g++.dg/modules/ben-1_a.C -std=c++17 (test for excess errors) FAIL: g++.dg/modules/ben-1_a.C -std=c++2a (internal compiler error: Segmentation fault) FAIL: g++.dg/modules/ben-1_a.C -std=c++2a (test for excess errors) FAIL: g++.dg/modules/ben-1_a.C -std=c++2b (internal compiler error: Segmentation fault) FAIL: g++.dg/modules/ben-1_a.C -std=c++2b (test for excess errors) FAIL: g++.dg/modules/ben-1_a.C module-cmi =partitions/module-import.mod (partitions/module-import.mod) FAIL: g++.dg/modules/ben-1_b.C -std=c++17 (internal compiler error: Segmentation fault) FAIL: g++.dg/modules/ben-1_b.C -std=c++17 (test for excess errors) FAIL: g++.dg/modules/ben-1_b.C -std=c++2a (internal compiler error: Segmentation fault) FAIL: g++.dg/modules/ben-1_b.C -std=c++2a (test for excess errors) FAIL: g++.dg/modules/ben-1_b.C -std=c++2b (internal compiler error: Segmentation fault) FAIL: g++.dg/modules/ben-1_b.C -std=c++2b (test for excess errors) FAIL: g++.dg/modules/ben-1_b.C module-cmi =module.mod (module.mod) FAIL: g++.dg/modules/gc-2_a.C -std=c++17 (internal compiler error: Segmentation fault) FAIL: g++.dg/modules/gc-2_a.C -std=c++17 (test for excess errors) FAIL: g++.dg/modules/gc-2_a.C -std=c++2a (internal compiler error: Segmentation fault) FAIL: g++.dg/modules/gc-2_a.C -std=c++2a (test for excess errors) FAIL: g++.dg/modules/gc-2_a.C -std=c++2b (internal compiler error: Segmentation fault) FAIL: g++.dg/modules/gc-2_a.C -std=c++2b (test for excess errors) FAIL: g++.dg/modules/gc-2_a.C module-cmi =map-1_a.nms (map-1_a.nms) UNRESOLVED: g++.dg/modules/map-1 -std=c++17 execute UNRESOLVED: g++.dg/modules/map-1 -std=c++17 link UNRESOLVED: g++.dg/modules/map-1 -std=c++2a execute UNRESOLVED: g++.dg/modules/map-1 -std=c++2a link UNRESOLVED: g++.dg/modules/map-1 -std=c++2b execute UNRESOLVED: g++.dg/modules/map-1 -std=c++2b link FAIL: g++.dg/modules/map-1_a.C -std=c++17 (internal compiler error: Segmentation fault) FAIL: g++.dg/modules/map-1_a.C -std=c++17 (test for excess errors) FAIL: g++.dg/modules/map-1_a.C -std=c++2a (internal compiler error: Segmentation fault) FAIL: g++.dg/modules/map-1_a.C -std=c++2a (test for excess errors) FAIL: g++.dg/modules/map-1_a.C -std=c++2b (internal compiler error: Segmentation fault) FAIL: g++.dg/modules/map-1_a.C -std=c++2b (test for excess errors) FAIL: g++.dg/modules/map-1_a.C module-cmi =map-1_a.nms (map-1_a.nms) FAIL: g++.dg/modules/map-1_b.C -std=c++17 (internal compiler error: Segmentation fault) FAIL: g++.dg/modules/map-1_b.C -std=c++17 (test for excess errors) FAIL: g++.dg/modules/map-1_b.C -std=c++2a (internal compiler error: Segmentation fault) FAIL: g++.dg/modules/map-1_b.C -std=c++2a (test for excess errors) FAIL: g++.dg/modules/map-1_b.C -std=c++2b (internal compiler error: Segmentation fault) FAIL: g++.dg/modules/map-1_b.C -std=c++2b (test for excess errors) FAIL: g++.dg/modules/map-2.C -std=c++17 at line 8 (test for errors, line 7) FAIL: g++.dg/modules/map-2.C -std=c++17 at line 9 (test for errors, line ) FAIL: g++.dg/modules/map-2.C -std=c++17 (internal compiler error: Segmentation fault) FAIL: g++.dg/modules/map-2.C -std=c++17 (test for excess errors) FAIL: g++.dg/modules/map-2.C -std=c++2a at line 8 (test for errors, line 7) FAIL: g++.dg/modules/map-2.C -std=c++2a at line 9 (test for errors, line ) FAIL: g++.dg/modules/map-2.C -std=c++2a (internal compiler error: Segmentation fault) FAIL: g++.dg/modules/map-2.C -std=c++2a (test for excess errors) FAIL: g++.dg/modules/map-2.C -std=c++2b at line 8 (test for errors, line 7) FAIL: g++.dg/modules/map-2.C -std=c++2b at line 9 (test for errors, line ) FAIL: g++.dg/modules/map-2.C -std=c++2b (internal compiler error: Segmentation fault) FAIL: g++.dg/modules/map-2.C -std=c++2b (test for excess errors) === Thanks, -- Maxim Kuvyrkov https://www.linaro.org > > [P1689R5]: https://isocpp.org/files/papers/P1689R5.html > > I've also added patches to include imported module CMI files and the > module mapper file as dependencies of the compilation. I briefly looked > into adding dependencies on response files as well, but that appeared to > need some code contortions to have a `class mkdeps` available before > parsing the comma
Re: [PR91598] Improve autoprefetcher heuristic in haifa-sched.c
Hi All, I've forgotten to commit this patch when it was approved 2 years ago. It still applies cleanly to the current mainline and I've retested it (bootstrap+regtest) on aarch64-linux-gnu and arm-linux-gnueabihf with no regressions. I'll commit this shortly. Regards, On Tue, 3 Sept 2019 at 19:55, Wilco Dijkstra wrote: > Hi Maxim, > > > > Autoprefetching heuristic is enabled only for cores that support it, > and isn't active for by default. > > > > It's enabled on most cores, including the default (generic). So we do > have to be > > careful that this doesn't regress any other benchmarks or do worse on > modern > > cores. > > I benchmarked your scheduler change on a few AArch64 machines, and it > either has > no effect or a positive effect on SPECFP with no major outliers (and only > minor > codesize differences). So I think your proposed patch is OK as is. > > Cheers, > Wilco > > -- Maxim Kuvyrkov www.linaro.org 0001-Improve-autoprefetcher-heuristic-partly-fix-regressi.patch Description: Binary data
Re: [PATCH] Couple of debug dump improvements to scheduler (no code-gen changes)
Hi Jeff, I've forgotten to commit these patches when they were approved 2 years ago. They still apply cleanly to the current mainline and I've retested them (bootstrap+regtest) on aarch64-linux-gnu and arm-linux-gnueabihf with no regressions. I'll commit these shortly. Regards, On Fri, 30 Aug 2019 at 01:57, Jeff Law wrote: > On 8/29/19 9:44 AM, Maxim Kuvyrkov wrote: > > Hi, > > > > The first patch adds ranking statistics for autoprefetcher heuristic. > > > > The second one makes it easier to diff scheduler debug dumps by adding > more context lines for diff at clock increments. > > > > OK to commit? > OK for both. > jeff > -- Maxim Kuvyrkov www.linaro.org 0003-Improve-diff-ability-of-scheduler-logs.patch Description: Binary data 0002-Add-missing-entry-for-rank_for_schedule-stats.patch Description: Binary data
Re: [PATCH] Add description of how testsuite parallelization works
> On Jul 2, 2021, at 5:44 PM, Christophe Lyon > wrote: > > > > On Fri, Jul 2, 2021 at 4:29 PM Jakub Jelinek via Gcc-patches > wrote: > On Fri, Jul 02, 2021 at 05:20:33PM +0300, Maxim Kuvyrkov wrote: > > Hi Jakub, > > > > Thanks for helping me on IRC with debugging testsuite problems. Does this > > write up look good? > > > Hi Maxim, > Thanks for adding this! > > There's a typo in your ChangeLog : > * lib/gcc-dg.exp: Add a comment. > should be lib/gcc-defs.exp > > but I think the new commit hooks will catch it. It did, and I fixed that. Thanks! -- Maxim Kuvyrkov https://www.linaro.org > > Christophe > > > LGTM, thanks. > > Jakub >
[PATCH] [contrib] Remove broken compareSumTests3 script
Hi Matthew, Hi Sebastian, This patch removes compareSumTests3, which appears to be broken. It tries to call compareSumFiles(), which is nowhere to be found and was never present in the GCC repo. OK to remove? Regards, -- Maxim Kuvyrkov https://www.linaro.org 0001-contrib-Remove-broken-compareSumTests3-script.patch Description: Binary data
Re: [r12-2132 Regression] FAIL: g++.dg/warn/Warray-bounds-20.C -std=gnu++98 note (test for warnings, line 55) on Linux/x86_64
> On 9 Jul 2021, at 02:35, sunil.k.pandey via Gcc-patches
> wrote:
>
> On Linux/x86_64,
>
> a110855667782dac7b674d3e328b253b3b3c919b is the first bad commit
> commit a110855667782dac7b674d3e328b253b3b3c919b
> Author: Martin Sebor
> Date: Wed Jul 7 14:05:25 2021 -0600
>
>Correct handling of variable offset minus constant in -Warray-bounds
> [PR100137]
>
> caused
Hi Martin,
I see these failing on aarch64-linux-gnu as well:
>
> FAIL: gcc.dg/Wstringop-overflow-47.c pr97027 (test for warnings, line 34)
> FAIL: gcc.dg/Wstringop-overflow-47.c pr97027 (test for warnings, line 37)
> FAIL: gcc.dg/Wstringop-overflow-47.c pr97027 (test for warnings, line 42)
FWIW, I don’t see these on aarch64-linux-gnu:
> FAIL: g++.dg/warn/Warray-bounds-20.C -std=gnu++14 note (test for warnings,
> line 38)
> FAIL: g++.dg/warn/Warray-bounds-20.C -std=gnu++14 note (test for warnings,
> line 55)
> FAIL: g++.dg/warn/Warray-bounds-20.C -std=gnu++17 note (test for warnings,
> line 38)
> FAIL: g++.dg/warn/Warray-bounds-20.C -std=gnu++17 note (test for warnings,
> line 55)
> FAIL: g++.dg/warn/Warray-bounds-20.C -std=gnu++2a note (test for warnings,
> line 38)
> FAIL: g++.dg/warn/Warray-bounds-20.C -std=gnu++2a note (test for warnings,
> line 55)
> FAIL: g++.dg/warn/Warray-bounds-20.C -std=gnu++98 note (test for warnings,
> line 38)
> FAIL: g++.dg/warn/Warray-bounds-20.C -std=gnu++98 note (test for warnings,
> line 55)
--
Maxim Kuvyrkov
https://www.linaro.org
>
> with GCC configured with
>
> ../../gcc/configure
> --prefix=/local/skpandey/gccwork/toolwork/gcc-bisect-master/master/r12-2132/usr
> --enable-clocale=gnu --with-system-zlib --with-demangler-in-ld
> --with-fpmath=sse --enable-languages=c,c++,fortran --enable-cet --without-isl
> --enable-libmpx x86_64-linux --disable-bootstrap
>
> To reproduce:
>
> $ cd {build_dir}/gcc && make check
> RUNTESTFLAGS="dg.exp=gcc.dg/Wstringop-overflow-47.c
> --target_board='unix{-m32\ -march=cascadelake}'"
> $ cd {build_dir}/gcc && make check
> RUNTESTFLAGS="dg.exp=gcc.dg/Wstringop-overflow-47.c
> --target_board='unix{-m64\ -march=cascadelake}'"
> $ cd {build_dir}/gcc && make check
> RUNTESTFLAGS="dg.exp=g++.dg/warn/Warray-bounds-20.C
> --target_board='unix{-m32}'"
> $ cd {build_dir}/gcc && make check
> RUNTESTFLAGS="dg.exp=g++.dg/warn/Warray-bounds-20.C
> --target_board='unix{-m32\ -march=cascadelake}'"
>
> (Please do not reply to this email, for question about this report, contact
> me at skpgkp2 at gmail dot com)
Re: disable -Warray-bounds in libgo (PR 101374)
> On 9 Jul 2021, at 09:16, Richard Biener via Gcc-patches > wrote: > > On Thu, Jul 8, 2021 at 8:02 PM Martin Sebor via Gcc-patches > wrote: >> >> Hi Ian, >> >> Yesterday's enhancement to -Warray-bounds has exposed a couple of >> issues in libgo where the code writes into an invalid constant >> address that the warning is designed to flag. >> >> On the assumption that those invalid addresses are deliberate, >> the attached patch suppresses these instances by using #pragma >> GCC diagnostic but I don't think I'm supposed to commit it (at >> least Git won't let me). To avoid Go bootstrap failures please >> either apply the patch or otherwise suppress the warning (e.g., >> by using a volatile pointer temporary). > > Btw, I don't think we should diagnose things like > >*(int*)0x21 = 0x21; > > when somebody literally writes that he'll be just annoyed by diagnostics. And we have an assortment of similar cases in 32-bit ARM kernel-page helpers. At the moment building libatomic for arm-linux-gnueabihf fails with: === In function ‘select_test_and_set_8’, inlined from ‘select_test_and_set_8’ at /home/tcwg-buildslave/workspace/tcwg-dev-build/snapshots/gcc.git~master/libatomic/tas_n.c:115:1: /home/tcwg-buildslave/workspace/tcwg-dev-build/snapshots/gcc.git~master/libatomic/config/linux/arm/host-config.h:42:34: error: array subscript 0 is outside array bounds of ‘unsigned int[0]’ [-Werror=array-bounds] 42 | #define __kernel_helper_version (*(unsigned int *)0x0ffc) | ~^~~~ === In libatomic/config/linux/arm/host-config.h we have: === /* Kernel helper for 32-bit compare-and-exchange. */ typedef int (__kernel_cmpxchg_t) (UWORD oldval, UWORD newval, UWORD *ptr); #define __kernel_cmpxchg (*(__kernel_cmpxchg_t *) 0x0fc0) /* Kernel helper for 64-bit compare-and-exchange. */ typedef int (__kernel_cmpxchg64_t) (const U_8 * oldval, const U_8 * newval, U_8 *ptr); #define __kernel_cmpxchg64 (*(__kernel_cmpxchg64_t *) 0x0f60) /* Kernel helper for memory barrier. */ typedef void (__kernel_dmb_t) (void); #define __kernel_dmb (*(__kernel_dmb_t *) 0x0fa0) /* Kernel helper page version number. */ #define __kernel_helper_version (*(unsigned int *)0x0ffc) === -- Maxim Kuvyrkov https://www.linaro.org
Re: [Linaro-TCWG-CI] gcc patch #75674: FAIL: 68 regressions
Hi Everyone, Normally, notifications from Linaro TCWG precommit CI are sent only to patch author and patch submitter. In this case the sender was rewritten to "Benjamin Priour via Gcc-patches ", which was detected by Patchwork [1] as patch submitter. Hi Mark, Is "From:" re-write on gcc-patches@ mailing list a side-effect of [2]? I see that some, but not all messages to gcc-patches@ have their "From:" re-written. Also, do you know if re-write of "From:" on gcc-patches@ is expected? [1] https://patchwork.sourceware.org/project/gcc/list/ [2] https://sourceware.org/bugzilla/show_bug.cgi?id=29713 Thanks! -- Maxim Kuvyrkov https://www.linaro.org > On Sep 12, 2023, at 02:58, ci_not...@linaro.org wrote: > > Dear contributor, our automatic CI has detected problems related to your > patch(es). Please find some details below. If you have any questions, > please follow up on linaro-toolch...@lists.linaro.org mailing list, Libera's > #linaro-tcwg channel, or ping your favourite Linaro toolchain developer on > the usual project channel. > > In CI config tcwg_gcc_check/master-aarch64 after: > > | gcc patch https://patchwork.sourceware.org/patch/75674 > | Author: benjamin priour > | Date: Mon Sep 11 19:44:34 2023 +0200 > | > | analyzer: Move gcc.dg/analyzer tests to c-c++-common (3) [PR96395] > | > | Hi, > | > | Patch below is mostly done, just have to check the formatting. > | Althought, I'd like your feedback on how to manage named_constants > | from enum in C++. > | ... 21 lines of the commit log omitted. > | ... applied on top of baseline commit: > | 048927ed856 i386: Handle CONST_WIDE_INT in output_pic_addr_const [PR111340] > > FAIL: 68 regressions > > regressions.sum: > === g++ tests === > > Running g++:g++.dg/analyzer/analyzer.exp ... > FAIL: c-c++-common/analyzer/call-summaries-2.c -std=c++14 (test for excess > errors) > FAIL: c-c++-common/analyzer/call-summaries-2.c -std=c++17 (test for excess > errors) > FAIL: c-c++-common/analyzer/call-summaries-2.c -std=c++20 (test for excess > errors) > FAIL: c-c++-common/analyzer/call-summaries-2.c -std=c++98 (test for excess > errors) > FAIL: c-c++-common/analyzer/memcpy-1.c -std=c++14 (test for warnings, line > 25) > FAIL: c-c++-common/analyzer/memcpy-1.c -std=c++14 (test for warnings, line > 48) > FAIL: c-c++-common/analyzer/memcpy-1.c -std=c++14 (test for excess errors) > ... and 63 more entries > > You can find the failure logs in *.log.1.xz files in > - > https://ci.linaro.org/job/tcwg_gcc_check--master-aarch64-precommit/2244/artifact/artifacts/artifacts.precommit/00-sumfiles/ > . > The full lists of regressions and progressions are in > - > https://ci.linaro.org/job/tcwg_gcc_check--master-aarch64-precommit/2244/artifact/artifacts/artifacts.precommit/notify/ > . > The list of [ignored] baseline and flaky failures are in > - > https://ci.linaro.org/job/tcwg_gcc_check--master-aarch64-precommit/2244/artifact/artifacts/artifacts.precommit/sumfiles/xfails.xfail > . > > > > -8<--8<--8<-- > The information below can be used to reproduce a debug environment: > > Current build : > https://ci.linaro.org/job/tcwg_gcc_check--master-aarch64-precommit/2244/artifact/artifacts > Reference build : > https://ci.linaro.org/job/tcwg_gcc_check--master-aarch64-build/927/artifact/artifacts
Re: PATCH v6 4/4] ree: Improve ree pass for rs6000 target using defined ABI interfaces.
Hi Ajit,
Is this patch supposed to be applied on top of another patch? As is, this
patch fails build on AArch64 and AArch32, and Linaro TCWG CI have sent
notifications about the failures for v5 [1] and v6 [2] of this patch to you.
Did you receive the notifications?
Kind regards,
[1]
https://patchwork.sourceware.org/project/gcc/patch/5ad7cdca-63e1-73af-b38d-d58898e21...@linux.ibm.com/
[2]
https://patchwork.sourceware.org/project/gcc/patch/65ed79a3-9964-dd50-39cb-98d5dbc72...@linux.ibm.com/
--
Maxim Kuvyrkov
https://www.linaro.org
> On Sep 18, 2023, at 09:59, Ajit Agarwal via Gcc-patches
> wrote:
>
> This new version of patch 6 use improve ree pass for rs6000 target using
> defined ABI interfaces.
> Bootstrapped and regtested on power64-linux-gnu.
>
> Review comments incorporated.
>
> Thanks & Regards
> Ajit
>
>
> ree: Improve ree pass for rs6000 target using defined abi interfaces
>
> For rs6000 target we see redundant zero and sign extension and done to
> improve ree pass to eliminate such redundant zero and sign extension
> using defined ABI interfaces.
>
> 2023-09-18 Ajit Kumar Agarwal
>
> gcc/ChangeLog:
>
> * ree.cc (combine_reaching_defs): Use of zero_extend and sign_extend
> defined abi interfaces.
> (add_removable_extension): Use of defined abi interfaces for no
> reaching defs.
> (abi_extension_candidate_return_reg_p): New function.
> (abi_extension_candidate_p): New function.
> (abi_extension_candidate_argno_p): New function.
> (abi_handle_regs_without_defs_p): New function.
> (abi_target_promote_function_mode): New function.
>
> gcc/testsuite/ChangeLog:
>
>* g++.target/powerpc/zext-elim-3.C
> ---
> gcc/ree.cc| 145 +-
> .../g++.target/powerpc/zext-elim-3.C | 13 ++
> 2 files changed, 155 insertions(+), 3 deletions(-)
> create mode 100644 gcc/testsuite/g++.target/powerpc/zext-elim-3.C
>
> diff --git a/gcc/ree.cc b/gcc/ree.cc
> index fc04249fa84..e395af6b1bd 100644
> --- a/gcc/ree.cc
> +++ b/gcc/ree.cc
> @@ -514,7 +514,8 @@ get_uses (rtx_insn *insn, rtx reg)
> if (REGNO (DF_REF_REG (def)) == REGNO (reg))
> break;
>
> - gcc_assert (def != NULL);
> + if (def == NULL)
> +return NULL;
>
> ref_chain = DF_REF_CHAIN (def);
>
> @@ -750,6 +751,118 @@ get_extended_src_reg (rtx src)
> return src;
> }
>
> +/* Return TRUE if target mode is equal to source mode of zero_extend
> + or sign_extend otherwise false. */
> +
> +static bool
> +abi_target_promote_function_mode (machine_mode mode)
> +{
> + int unsignedp;
> + machine_mode tgt_mode =
> +targetm.calls.promote_function_mode (NULL_TREE, mode, &unsignedp,
> + NULL_TREE, 1);
> +
> + if (tgt_mode == mode)
> +return true;
> + else
> +return false;
> +}
> +
> +/* Return TRUE if the candidate insn is zero extend and regno is
> + an return registers. */
> +
> +static bool
> +abi_extension_candidate_return_reg_p (rtx_insn *insn, int regno)
> +{
> + rtx set = single_set (insn);
> +
> + if (GET_CODE (SET_SRC (set)) != ZERO_EXTEND)
> +return false;
> +
> + if (FUNCTION_VALUE_REGNO_P (regno))
> +return true;
> +
> + return false;
> +}
> +
> +/* Return TRUE if reg source operand of zero_extend is argument registers
> + and not return registers and source and destination operand are same
> + and mode of source and destination operand are not same. */
> +
> +static bool
> +abi_extension_candidate_p (rtx_insn *insn)
> +{
> + rtx set = single_set (insn);
> +
> + if (GET_CODE (SET_SRC (set)) != ZERO_EXTEND)
> +return false;
> +
> + machine_mode ext_dst_mode = GET_MODE (SET_DEST (set));
> + rtx orig_src = XEXP (SET_SRC (set),0);
> +
> + bool copy_needed
> += (REGNO (SET_DEST (set)) != REGNO (XEXP (SET_SRC (set), 0)));
> +
> + if (!copy_needed && ext_dst_mode != GET_MODE (orig_src)
> + && FUNCTION_ARG_REGNO_P (REGNO (orig_src))
> + && !abi_extension_candidate_return_reg_p (insn, REGNO (orig_src)))
> +return true;
> +
> + return false;
> +}
> +
> +/* Return TRUE if the candidate insn is zero extend and regno is
> + an argument registers. */
> +
> +static bool
> +abi_extension_candidate_argno_p (rtx_code code, int regno)
> +{
> + if (code != ZERO_EXTEND)
> +return false;
> +
> + if (FUNCTION_ARG_REGNO_P (regno))
> +return true;
> +
> + return false;
> +}
> +
> +/* Return TRUE if the candidate insn doesn't have defs and have
> + * uses without RTX_BIN_ARITH/RTX_COMM_ARITH/RTX_UNARY rtx class. */
> +
> +static bool
> +abi_handle_regs_without_defs_p (rtx_insn *insn)
> +{
> + if (side_effects_p (PATTERN (insn)))
> +return false;
> +
> + struct df_link *uses = get_uses (insn, SET_DEST (PATTERN (insn)));
> +
> + if (!uses)
> +return false;
> +
> + for (df_link *use = uses; use; use = use->next)
> +{
> + if (!use->ref)
> + return false;
> +
> + if (BLOCK_FOR_INSN (insn) != BLOCK_FOR_INSN (DF_REF_INSN (use->ref)))
> + return false;
> +
> + rtx_insn *use_ins
Re: [PATCH] middle-end/94188 fix fold of addr expression generation
> On 17 Mar 2020, at 17:40, Richard Biener wrote: > > > This adds a missing type conversion to build_fold_addr_expr and adjusts > fallout - build_fold_addr_expr was used as a convenience to build an > ADDR_EXPR but some callers do not expect the result to be simplified > to something else. > > Bootstrapped on x86_64-unknown-linux-gnu, testin in progress. > > This is the 3rd or 4th attempt and I hope to have catched all fallout > with this. I think it's inevitable we fix the mistake in > build_fold_addr_expr. > > Richard. > > 2020-03-17 Richard Biener > > PR middle-end/94188 > * fold-const.c (build_fold_addr_expr): Convert address to > correct type. > * asan.c (maybe_create_ssa_name): Strip useless type conversions. > * gimple-fold.c (gimple_fold_stmt_to_constant_1): Use build1 > to build the ADDR_EXPR which we don't really want to simplify. > * tree-ssa-dom.c (record_equivalences_from_stmt): Likewise. > * tree-ssa-loop-im.c (gather_mem_refs_stmt): Likewise. > * tree-ssa-forwprop.c (forward_propagate_addr_expr_1): Likewise. > (simplify_builtin_call): Strip useless type conversions. > * tree-ssa-strlen.c (new_strinfo): Likewise. > > * gcc.dg/pr94188.c: New testcase. Hi Richard, This breaks Linux kernel build on 32-bit ARM: 00:01:29 ./include/linux/string.h:333:9: internal compiler error: in gen_movsi, at config/arm/arm.md:6291 00:01:29 make[2]: *** [sound/drivers/serial-u16550.o] Error 1 Would you please investigate? Let me know if you need any help reproducing the problem. Kernel’s build line is (assuming cross-compilation): make CC=/path/to/arm-linux-gnueabihf-gcc ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- HOSTCC=gcc allyesconfig Regards, -- Maxim Kuvyrkov https://www.linaro.org
Re: [PATCH] tree-optimization PR/101014 - Limit new value calculations to first order effects.
> On 15 Jun 2021, at 00:07, Andrew MacLeod via Gcc-patches
> wrote:
>
> As mentioned in the Text from the PR:
>
> "When a range is being calculated for an ssa-name, the propagation process
> often goes along back edges. These back edges sometime require other
> ssa-names which have not be processed yet. These are flagged as "poor values"
> and when propagation is done, we visit the list of poor values, calculate
> them, and see if that may result if a better range for the original ssa-name.
>
> The problem is that calculating these poor values may also spawn another set
> of requests since the block at the far end of the back edge has not been
> processed yet... its highly likely that some additional unprocessed ssa-names
> are used in the calculation of that name, but typically they do not affect
> the current range in a significant way.
>
> Thus we mostly we care about the first order effect only. It turns out to be
> very rare that a 2nd order effect on a back edge affects anything that we
> don't catch later.
>
> This patch turns off poor-value tagging when looking up the first order
> values, thus avoiding the 2nd order and beyond cascading effects.
>
> I haven't found a test case we miss yet because of this change, yet it
> probably resolves a number of the outstanding compilation problems in a
> significant way.
>
> I think this will probably apply to gcc 11 in some form as well, so I'll look
> at an equivalent patch for there."
>
>
> This patch simplifies the enable_new_value routines.. replacing the
> enable/disable with an enable with flag routine, which returns the previous
> value.This lets us change the mode and then set it back to what it was
> before. Seems better in general.
>
> Then disables new values for 2nd+ order effects. GCC11 patch forthcoming.
>
> Bootstraps on x86_64-pc-linux-gnu, no regressions. pushed.
>
> Andrew
Hi Andrew,
This causes bootstrap-with-ubsan failure on at least aarch64-linux-gnu, likely,
others:
# 00:42:32
/home/tcwg-buildslave/workspace/tcwg_gnu_0/abe/snapshots/gcc.git~master/gcc/gimple-range-cache.cc:757:8:
runtime error: load of value 48, which is not a valid value for type 'bool'
# 00:42:32
/home/tcwg-buildslave/workspace/tcwg_gnu_0/abe/snapshots/gcc.git~master/gcc/gimple-range-cache.cc:757:8:
runtime error: load of value 48, which is not a valid value for type 'bool'
# 00:42:32
/home/tcwg-buildslave/workspace/tcwg_gnu_0/abe/snapshots/gcc.git~master/gcc/gimple-range-cache.cc:757:8:
runtime error: load of value 32, which is not a valid value for type 'bool'
# 00:42:32
/home/tcwg-buildslave/workspace/tcwg_gnu_0/abe/snapshots/gcc.git~master/gcc/gimple-range-cache.cc:757:8:
runtime error: load of value 48, which is not a valid value for type 'bool'
# 00:42:32
/home/tcwg-buildslave/workspace/tcwg_gnu_0/abe/snapshots/gcc.git~master/gcc/gimple-range-cache.cc:757:8:
runtime error: load of value 32, which is not a valid value for type 'bool'
# 00:42:32
/home/tcwg-buildslave/workspace/tcwg_gnu_0/abe/snapshots/gcc.git~master/gcc/gimple-range-cache.cc:757:8:
runtime error: load of value 48, which is not a valid value for type 'bool'
# 00:42:32
/home/tcwg-buildslave/workspace/tcwg_gnu_0/abe/snapshots/gcc.git~master/gcc/gimple-range-cache.cc:757:8:
runtime error: load of value 32, which is not a valid value for type 'bool'
# 00:42:32
/home/tcwg-buildslave/workspace/tcwg_gnu_0/abe/snapshots/gcc.git~master/gcc/gimple-range-cache.cc:757:8:
runtime error: load of value 32, which is not a valid value for type 'bool'
>
> @@ -748,21 +748,15 @@ ranger_cache::dump (FILE *f)
>fprintf (f, "\n");
> }
>
> -// Allow the cache to flag and query new values when propagation is forced
> -// to use an unknown value.
> +// Allow or disallow the cache to flag and query new values when propagation
> +// is forced to use an unknown value. The previous state is returned.
>
> -void
> -ranger_cache::enable_new_values ()
> -{
> - m_new_value_p = true;
> -}
> -
> -// Disable new value querying.
> -
> -void
> -ranger_cache::disable_new_values ()
> +bool
> +ranger_cache::enable_new_values (bool state)
> {
> - m_new_value_p = false;
> + bool ret = m_new_value_p;
I think changing this to
bool ret = (bool) m_new_value_p;
might be enough, but you know this code better.
Would you please take a look at this?
> + m_new_value_p = state;
> + return ret;
> }
>
> // Dump the caches for basic block BB to file F.
Thanks,
--
Maxim Kuvyrkov
https://www.linaro.org
Re: [PATCH] Avoid loading an undefined value in the ranger_cache constructor.
> On 16 Jun 2021, at 20:14, Andrew MacLeod wrote: > > On 6/16/21 5:41 AM, Maxim Kuvyrkov wrote: >> >>> + m_new_value_p = state; >>> + return ret; >>> } >>>// Dump the caches for basic block BB to file F. >> Thanks, >> >> -- >> Maxim Kuvyrkov >> https://www.linaro.org >> > Let me know if the problem is resolved. > > pushed as obvious. > Hi Andrew, All good, thanks! CI is back to green. -- Maxim Kuvyrkov https://www.linaro.org
[PATCH] Add description of how testsuite parallelization works
Hi Jakub, Thanks for helping me on IRC with debugging testsuite problems. Does this write up look good? Regards, -- Maxim Kuvyrkov https://www.linaro.org 0001-Add-description-of-how-testsuite-parallelization-wor.patch Description: Binary data
