[BOLT] Improve handling in BOLTBuilder and cleanup (#480)

- Add a clean-nfc-check step to remove auxiliary files before
nfc-mode-setup.
- Escalate errors in nfc-check-setup.py as warnings. Checks continue as
expected.
- Allow tests to proceed if the old/new binaries are not generated.
(rare case)
- Provide clearer message when GNU build IDs are embedded in the binary
but they do match. Tests are skip accordingly.

Author: paschalis-mpeis

zorg/buildbot/builders/BOLTBuilder.py

@@ -99,54 +99,82 @@ def getBOLTCmakeBuildFactory(
         # Individual markers to skip either in-tree or out-of-tree tests.
         skipInTree = ".llvm-bolt.skip.in-tree"
         skipOutOfTree = ".llvm-bolt.skip.out-of-tree"
+        boltNew = "bin/llvm-bolt.new"
+        boltOld = "bin/llvm-bolt.old"
 
         f.addSteps([
+            # Cleanup binaries and markers from previous NFC-mode runs.
+            ShellCommand(
+                name='clean-nfc-check',
+                command=(
+                        f"rm -f {boltNew} {boltOld} {hasSrcChanges} {skipInTree} {skipOutOfTree}; "
+                        ),
+                description=('Cleanup for NFC-Mode testing'),
+                descriptionDone=["NFC-Mode cleanup"],
+                haltOnFailure=False,
+                flunkOnFailure=False,
+                env=env),
+            # Build the current and previous revision of llvm-bolt.
             ShellCommand(
                 name='nfc-check-setup',
                 command=[
-                         f"../{f.monorepo_dir}/bolt/utils/nfc-check-setup.py",
+                        f"../{f.monorepo_dir}/bolt/utils/nfc-check-setup.py",
                         "--switch-back",
                         "--check-bolt-sources"
                         ],
                 description=('Setup NFC testing'),
                 descriptionDone=["NFC-Mode setup"],
                 warnOnFailure=True,
+                warnOnWarnings=True,
+                decodeRC={0: SUCCESS, 1: WARNINGS},
                 haltOnFailure=False,
                 flunkOnFailure=False,
                 env=env),
+            # Validate that NFC-mode comparison is meaningful by checking:
+            # - the old and new binaries exist
+            # - no unique IDs are embedded in the binaries
+            # Warns but does not fail when validation fails.
             ShellCommand(
                 name='nfc-check-validation',
                 command=(
-                    "info=$(bin/llvm-bolt.new --version | grep 'BOLT revision' "
+                    f"( ! [ -f {boltNew} ] || ! [ -f {boltOld} ] ) && "
+                    "{ printf 'NFC-Mode WARNING: old/new files not present. Tests will run.'; return 2; }; "
+                    f"uids=$({boltNew} --version | grep 'BOLT revision' "
                     "| grep -q '<unknown>' || echo 'bolt-revision'); "
-                    "info=$info$(readelf --notes bin/llvm-bolt.new "
+                    f"uids=$uids$(readelf --notes {boltNew} "
                     "| grep -q 'Build ID:' && echo ' GNU-build-id'); "
-                    "info=$(echo \"$info\" | sed 's/^ //'); "
-                    "[ ! -z \"$info\" ] || return 0 && "
-                    "(printf \"NFC-Mode WARNING: unique IDs found in binaries ($info)"
-                    ". This means tests will run at all times.\"; return 2)"
+                    "uids=$(echo \"$uids\" | sed 's/^ //'); "
+                   f"[ $(readelf --note {boltNew} {boltOld} | grep 'Build ID' | uniq -c | wc -l) -eq 1 ] "
+                    "&& extra='Identical build-ids.' || extra='Tests will run.'; "
+                    "[ ! -z \"$uids\" ] || return 0 && "
+                    "{ printf \"NFC-Mode WARNING: unique IDs found in binaries ($uids). $extra\"; return 2; }"
                     ),
                 description=('Check that nfc-mode works as intended when '
                              'comparing with the previous commit.'),
                 haltOnFailure=False,
                 warnOnFailure=True,
                 warnOnWarnings=True,
                 decodeRC={0: SUCCESS, 1: FAILURE, 2: WARNINGS},
-                descriptionDone=["NFC-Mode unique IDs in binaries"],
+                descriptionDone=["NFC-Mode Validation"],
                 env=env),
+            # Compare the current and previous llvm-bolt binaries. If they are
+            # identical, skip the following tests. If relevant source code
+            # changes are detected, still run the in-tree tests.
             ShellCommand(
                 name='nfc-check-bolt-different',
                 command=(
-                          f'rm -f {skipInTree} {skipOutOfTree}; '
-                          f'cmp -s bin/llvm-bolt.old bin/llvm-bolt.new && ('
+                          f'cmp -s {boltNew} {boltOld} && ('
                           f'touch {skipInTree}; touch {skipOutOfTree}); '
-                          f'[ -f {hasSrcChanges} ] && rm -f {skipInTree};'
-                          f'rm -f {hasSrcChanges}; '
+                          f'[ -f {hasSrcChanges} ] && rm -f {skipInTree}; '
+                          f'return 0'
                          ),
                 description=('Check if llvm-bolt binaries are different and '
                              'skip the following nfc-check steps'),
+                decodeRC={0: SUCCESS, 1: WARNINGS},
                 haltOnFailure=False,
                 env=env),
+            # Run in-tree tests if the llvm-bolt binary has changed, or if
+            # relevant source code changes are detected.
             LitTestCommand(
                 name='nfc-check-bolt',
                 command=["ninja", "check-bolt"],
@@ -157,6 +185,7 @@ def getBOLTCmakeBuildFactory(
                 flunkOnFailure=True,
                 doStepIf=FileDoesNotExist(f"build/{skipInTree}"),
                 env=env),
+            # Run out-of-tree large tests if the llvm-bolt binary has changed.
             LitTestCommand(
                 name='nfc-check-large-bolt',
                 command=['bin/llvm-lit', '-sv', '-j2',
@@ -171,6 +200,7 @@ def getBOLTCmakeBuildFactory(
             ])
 
     return f
+
 class SkipAwareBuild(Build):
     """
     Custom Build class that marks the overall build status as skipped when no