Closed Bug 1443827 Opened 2 years ago Closed 2 years ago

Update clang-cl toolchain to a 7.0.0 trunk revision

Categories

(Firefox Build System :: General, enhancement)

3 Branch
enhancement
Not set

Tracking

(firefox60 fixed)

RESOLVED FIXED
mozilla60
Tracking Status
firefox60 --- fixed

People

(Reporter: dmajor, Assigned: dmajor)

References

Details

Attachments

(2 files)

I want to have bootstrap provide a newer clang before I send around an invitation for developers to use clang-cl locally.

Unfortunately, newer revisions break our clang-plugin builds (bug 1427808) and I've spent too much time trying to work around that and can't afford to be blocked anymore.

I propose to create clang-win{32,64}-st-an.json files to keep the static analysis builders on their current revision, and update the primary json's to a recent 7.0.0 trunk.
Blocks: 1443590
Blocks: WinLTO
When will we be able to use actual clang releases for clang-cl?
With this change, nobody else is using the plain win32-clang-cl toolchain, so I've removed it.
Assignee: nobody → dmajor
Attachment #8957135 - Flags: review?(nfroyd)
Hm, splinter didn't do a great job with that. The patch also has some moves and copies-with-changes:

rename from build/build-clang/clang-win32.json
rename to build/build-clang/clang-win32-st-an.json

copy from build/build-clang/clang-win64.json
copy to build/build-clang/clang-win64-st-an.json

rename from taskcluster/scripts/misc/build-clang32-windows.sh
rename to taskcluster/scripts/misc/build-clang32-st-an-windows.sh
--- a/taskcluster/scripts/misc/build-clang32-windows.sh
+++ b/taskcluster/scripts/misc/build-clang32-st-an-windows.sh
@@ -1,3 +1,3 @@
 #!/bin/bash
 
-source build/src/taskcluster/scripts/misc/build-clang-windows-helper32.sh clang-win32.json
+source build/src/taskcluster/scripts/misc/build-clang-windows-helper32.sh clang-win32-st-an.json

copy from taskcluster/scripts/misc/build-clang64-windows.sh
copy to taskcluster/scripts/misc/build-clang64-st-an-windows.sh
--- a/taskcluster/scripts/misc/build-clang64-windows.sh
+++ b/taskcluster/scripts/misc/build-clang64-st-an-windows.sh
@@ -1,3 +1,3 @@
 #!/bin/bash
 
-source build/src/taskcluster/scripts/misc/build-clang-windows-helper64.sh clang-win64.json
+source build/src/taskcluster/scripts/misc/build-clang-windows-helper64.sh clang-win64-st-an.json
Attachment #8957136 - Flags: review?(nfroyd)
(In reply to Mike Hommey [:glandium] from comment #1)
> When will we be able to use actual clang releases for clang-cl?

Is there any reason that we can't use clang-cl 6.0 as the base revision for this new job?
Flags: needinfo?(dmajor)
(In reply to Mike Hommey [:glandium] from comment #1)
> When will we be able to use actual clang releases for clang-cl?

When the performance and (since we're offering this as bootstrap's recommended compiler) the developer experience are acceptable.

(In reply to Nathan Froyd [:froydnj] from comment #5)
> Is there any reason that we can't use clang-cl 6.0 as the base revision for
> this new job?

The first thing that comes to mind is support for order files in lld, which gives us 3-4% improvement in the absence of PGO. While it's a small enough patch [1] that we couple apply it to 6.0.0 locally, it is very likely that we'll need additional fixes in multiple sub-repos to match MSVC speeds (see bug 1443590 comment 8).

Also lld currently doesn't support incremental linking (not even on trunk). And I hear 6.0.0 has a ton of PDB improvements but it's possible there may be more room for improvement there.

[1] https://github.com/llvm-mirror/lld/commit/ae1d72fc3b4e7ef091a67a9f1fba849d5c6cad7d#diff-4fa2ac5353638195d4ce3bc48a5a9db7
Flags: needinfo?(dmajor)
Comment on attachment 8957136 [details] [diff] [review]
Update clang-cl builders to r326909 and add the lld repo

Review of attachment 8957136 [details] [diff] [review]:
-----------------------------------------------------------------

r=me assuming the comment below is correct.

::: build/build-clang/clang-win64.json
@@ -15,5 @@
>      "patches": [
> -      "r318309.patch",
> -      "loosen-msvc-detection.patch",
> -      "hide-gcda-profiling-symbols.patch",
> -      "fflush-before-unlocking.patch"

I am assuming that all of the patches that you are removing made it into the clang tree at some point?
Comment on attachment 8957135 [details] [diff] [review]
Move clang-cl static analysis builds to their own toolchain task

Review of attachment 8957135 [details] [diff] [review]:
-----------------------------------------------------------------

Makes sense; it's a little unfortunate that we now have even *more* profusion of clang builds. :(
Attachment #8957135 - Flags: review?(nfroyd) → review+
(In reply to Nathan Froyd [:froydnj] from comment #7)
> Comment on attachment 8957136 [details] [diff] [review]
> Update clang-cl builders to r326909 and add the lld repo
> 
> Review of attachment 8957136 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=me assuming the comment below is correct.
> 
> ::: build/build-clang/clang-win64.json
> @@ -15,5 @@
> >      "patches": [
> > -      "r318309.patch",
> > -      "loosen-msvc-detection.patch",
> > -      "hide-gcda-profiling-symbols.patch",
> > -      "fflush-before-unlocking.patch"
> 
> I am assuming that all of the patches that you are removing made it into the
> clang tree at some point?

Yep. r318309, I just assumed. The other two I checked.
Comment on attachment 8957136 [details] [diff] [review]
Update clang-cl builders to r326909 and add the lld repo

I thought I r+'d this, but apparently not.
Attachment #8957136 - Flags: review?(nfroyd) → review+
Pushed by dmajor@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/29e699833412
Move clang-cl static analysis builds to their own toolchain task. r=froydnj
https://hg.mozilla.org/integration/mozilla-inbound/rev/8e1184c068ad
Update clang-cl builders to r326909 and add the lld repo. r=froydnj
https://hg.mozilla.org/mozilla-central/rev/29e699833412
https://hg.mozilla.org/mozilla-central/rev/8e1184c068ad
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/89edbe9d2bf2
Port bug 1443827 to TB: Move clang-cl static analysis builds to their own toolchain task. rs=bustage-fix
Depends on: 1444591
Version: Version 3 → 3 Branch
You need to log in before you can comment on or make changes to this bug.