[BOLT] Guard llvm-bolt-wrapper logic of NFC-Mode behind a flag

[paschalis-mpeis]

Buildbot (BOLTBuilder) no longer relies on a wrapper script to run tests. This
patch guards the wrapper logic under a flag that is disabled by default. This
it allows to:

• Eliminate the need for special handling in some tests.
• Fix the issue of a wrapper loop (described below)
• Simplify the NFC-Mode setup.

Background:
Previously, tests ran unconditionally, which also compiled any missing utilities
and the unit tests.

The nfc-check-setup.py created:

• llvm-bolt.new, renamed from the current compilation
• llvm-bolt.old, built from the previous SHA
• llvm-bolt: a python wrapper pointing to llvm-bolt.new

Current behaviour and wrapper issue:
As before, the old/new binaries identify whether a patch affects BOLT. If so,
ninja check-bolt builds missing dependencies and run tests, overwriting the
llvm-bolt wrapper with a binary.

However, if Ninja reports:

ninja: no work to do.

the wrapper remains in place. If the next commit also does no work,
nfc-check-setup.py renames the existing wrapper to llvm-bolt.new, causing an
infinite loop.

Allowing to disable the wrapper logic prevents this scenario and simplifies the flow.

Test plan:

Creates llvm-bolt.new and llvm-bolt.old and stays on previous revision:

./nfc-check-setup.py build

Creates llvm-bolt.new and llvm-bolt.old and returns on current revision:

./nfc-check-setup.py build --switch-back

Creates llvm-bolt.new and llvm-bolt.old, returns on current revision, and
creates a wrapper:

./nfc-check-setup.py build --switch-back --create-wrapper

Creates llvm-bolt.new and llvm-bolt.old, and passes an invalid argument to the
wrapper:

./nfc-check-setup.py build --switch-back --create-wrapper --random-arg

[llvmbot]

@llvm/pr-subscribers-bolt

Author: Paschalis Mpeis (paschalis-mpeis)

Changes

Buildbot (BOLTBuilder) no longer relies on a wrapper script to run tests. This
patch removes the wrapper logic to:

• Eliminate the need for special handling in some tests.
• Fix the issue of a wrapper loop (described below)
• Simplify the NFC-Mode setup.

Background:
Previously, tests ran unconditionally, which also compiled any missing utilities
and the unit tests.

The nfc-check-setup.py created:

• llvm-bolt.new, renamed from the current compilation
• llvm-bolt.old, built from the previous SHA
• llvm-bolt: a python wrapper pointing to llvm-bolt.new

Current behaviour and wrapper issue:
As before, the old/new binaries identify whether a patch affects BOLT. If so,
ninja check-bolt builds missing dependencies and run tests, overwriting the
llvm-bolt wrapper with a binary.

However, if Ninja reports:

ninja: no work to do.

the wrapper remains in place. If the next commit also does no work,
nfc-check-setup.py renames the existing wrapper to llvm-bolt.new, causing an
infinite loop.

Removing the wrapper logic prevents this scenario and simplifies the flow.

---

Full diff: https://github.com/llvm/llvm-project/pull/146209.diff

1 Files Affected:

• (modified) bolt/utils/nfc-check-setup.py (+2-13)

diff --git a/bolt/utils/nfc-check-setup.py b/bolt/utils/nfc-check-setup.py
index 275ac7b886d00..9e4fc8bc101b1 100755
--- a/bolt/utils/nfc-check-setup.py
+++ b/bolt/utils/nfc-check-setup.py
@@ -48,8 +48,7 @@ def main():
         description=textwrap.dedent(
             """
             This script builds two versions of BOLT (with the current and
-            previous revision) and sets up symlink for llvm-bolt-wrapper.
-            Passes the options through to llvm-bolt-wrapper.
+            previous revision).
             """
         )
     )
@@ -76,7 +75,7 @@ def main():
         default="HEAD^",
         help="Revision to checkout to compare vs HEAD",
     )
-    args, wrapper_args = parser.parse_known_args()
+    args = parser.parse_args()
     bolt_path = f"{args.build_dir}/bin/llvm-bolt"
 
     source_dir = None
@@ -90,7 +89,6 @@ def main():
         sys.exit("Source directory is not found")
 
     script_dir = os.path.dirname(os.path.abspath(__file__))
-    wrapper_path = f"{script_dir}/llvm-bolt-wrapper.py"
     # build the current commit
     subprocess.run(
         shlex.split("cmake --build . --target llvm-bolt"), cwd=args.build_dir
@@ -131,15 +129,6 @@ def main():
     )
     # rename llvm-bolt
     os.replace(bolt_path, f"{bolt_path}.old")
-    # set up llvm-bolt-wrapper.ini
-    ini = subprocess.check_output(
-        shlex.split(f"{wrapper_path} {bolt_path}.old {bolt_path}.new") + wrapper_args,
-        text=True,
-    )
-    with open(f"{args.build_dir}/bin/llvm-bolt-wrapper.ini", "w") as f:
-        f.write(ini)
-    # symlink llvm-bolt-wrapper
-    os.symlink(wrapper_path, bolt_path)
     if args.switch_back:
         if stash:
             subprocess.run(shlex.split("git stash pop"), cwd=source_dir)

[aaupov]

Put the symlinking logic under an option, disabled in a buildbot?
nfc-check-setup is handy for bisecting (where llvm-bolt binary is rebuilt and the script needs to be symlinked in its place) as well as for launching comparisons manually.

[github-actions]

✅ With the latest revision this PR passed the Python code formatter.

[paschalis-mpeis]

Hey Amir,

Thanks for providing use cases for the wrapper.
Introduced --create-wrapper and some examples at the end of the description of this PR.