[BOLT] Set nfc-tests once

[paschalis-mpeis]

When setting up the nfc-mode tests, avoid adding an extra pair of in-tree and
out-of-tree tests that were unconditional. This was previously done with
addNinjaSteps. Instead, add those later and make them conditional on llvm-bolt
being modified.

Since tests are now only added by BOLTBuilder:

• ninja runs the in-tree tests to correctly build dependencies.
• flunkOnFailure is set to update the build status on failures.

Some nits:

• the '--switch-back' flag is used (from nfc-check-setup.py).
• a 'nfc-' prefix is appended to 'check-bolt-different' step
• remove timing.log cleanup

[paschalis-mpeis]

Adding an example to this:

• Patch 90beda is an unrelated change.
• Normally, none of the tests should have run.
• However, both in-tree and out-of-tree tests run twice (build: 20205):
  • first run unconditionally by steps 8 and 9
  • then they run conditionally at steps 12 and 13.

This patch will add them once, under the NFC-mode condition. After #142410 is merged, those tests won't run.

[gkistanova]

Thanks for the patch, Paschalis!

Could you make the change to UnifiedTreeBuilder a separate PR, please?

Or you can pass it an empty list instead of None as that's what UnifiedTreeBuilder expects in this case.

[paschalis-mpeis]

Hi Galina,

Thank you for your review. I've created the below PR as you suggested:

• Handle None checks in UnifiedTreeBuilder.addNinjaSteps #477

The current PR (#466) is now based on #477 and includes only BOLTBuilder logic.
Let me know if you have any further comments.

---

On the BOLTBuilder logic itself, any comments from @aaupov ?

[paschalis-mpeis]

Thanks Galina for approving #477. I now assume there are no objections on your end on this PR.

[aaupov]

I'm afraid running check-bolt after nfc-check-setup will rebuild BOLT and replace the llvm-bolt symlink to llvm-bolt-wrapper. Did you check it works?

[paschalis-mpeis]

Hey Amir,

Thanks for the review. Yes, all patches have been tested for some time.

Why we need check-bolt now:

Previously, addNinjaSteps ran check-bolt unconditionally, building any required utilities, and running the tests.
Now that we do this step conditionally, so this needs to be in place.

About the llvm-bolt-wrapper symlink:

Indeed, the nfc-check-setup creates a symlink to llvm-bolt-wrapper, which is later replaced by the check-bolt. However, I don't think this affects the BOLT builder now:

• step nfc-check-setup:
  • provides two binaries (old, new)
  • does --switch-back to the commit we are interested in
  • creates that symlink
• step nfc-check-bolt-different: determines whether to run tests
• step nfc-check-bolt: conditionally runs as needed, doing check-bolt and overwriting that symlink
• step nfc-check-large-bolt: similarly with above

So, tests do run on the correct llvm-bolt binary version, which is not a symlink. Does the llvm-bolt-wrapper serve any purpose, at least for llvm-zorg, other than the nfc-stat-parser.py comparisons, now cleaned up by #465 ?

PS: I'm happy to perform additional cleanup on the llvm-project tools after we merge these changes.