Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add WoA testsuite buildbot #253

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

omjavaid
Copy link
Contributor

This is a follow up patch on #245 and #252
It adds a new builder called clang-arm64-windows-msvc-testsuite. It will
do stage1 and stage2 builds without running checks. Checks are already
covered by single stage and 2 stage bots.

This updates LLVM WoA bots with following changes:
1) Do not run stage1 testing on 2 stage clang bot.
2) Remove redundant workers.
3) Remove -DCOMPILER_RT_BUILD_PROFILE=OFF
4) Add compiler-rt to single stage bot
5) Set clean=False for the 2stage bots
6) Set "-DCLANG_DEFAULT_LINKER=lld" for all WoA bots
@DavidSpickett
Copy link
Contributor

Checks are already covered by single stage and 2 stage bots.

What is the reason to not add this onto the 2 stage bots?

I can think of a few:

  • The 2 stage bot takes approximately a human lifetime already :)
  • The results from each bot will match the name (test suite failures from testsuite, stage 1 checks from single stage etc.)

Any more?

Assuming we have the hardware resource I agree with the decision.

@@ -591,22 +591,42 @@
"-DMLIR_RUN_ARM_SVE_TESTS=True",
"-DLLVM_LIT_ARGS='-v'"])},

{'name' : "clang-arm64-windows-msvc-testsuite",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is very late to say this, but it is confusing that we have "msvc" in the bot names. Could we change that for this new one? clang-arm64-windows-testsuite would be fine.

A couple of times I've had to explain that the "msvc" bot actually uses clang-cl.

I would have proposed changing all the names but that's a fiddly multi week process so I never had the energy to do it.

checkout_lld=True,
runTestSuite=True,
testWithLNT=False,
testsuite_flags=["-DTEST_SUITE_SUBDIRS='Fortran'"],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does enabling flang not just automatically do this?

# so, before that's fixed, disable everything that triggers its build.
"-DCOMPILER_RT_BUILD_SANITIZERS=OFF",
"-DCOMPILER_RT_BUILD_PROFILE=OFF"])},
"-DCOMPILER_RT_BUILD_SANITIZERS=OFF"])},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Windows on Arm, we are only building compiler-rt for the compiler runtime part, correct? I see that all these are disabling the sanitizers.

@@ -41,7 +41,6 @@ def get_all():
create_worker("linaro-g3-04", max_builds=1),

# AArch64 Windows Microsoft Surface X Pro
create_worker("linaro-armv8-windows-msvc-01", max_builds=1),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Be careful with the timing of when this change lands vs. when you remove the agent from the actual machine. I managed to get our server IP banned because I left some workers running that kept trying to connect to the build master.

If the machine is offline before this lands there should be no problem.

'builddir': "clang-arm64-windows-msvc-testsuite",
'factory' : ClangBuilder.getClangCMakeBuildFactory(
vs="manual",
checks=[],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where are you disabling the check stage 1 and 2 here? Also this doesn't seem to be a 2 stage, there's no useTwoStage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants