Closed
Bug 1316537
Opened 9 years ago
Closed 9 years ago
Add 64-bit clang-cl toolchain builds to taskcluster task definitions
Categories
(Firefox Build System :: Task Configuration, task)
Firefox Build System
Task Configuration
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: ting, Assigned: ting)
Details
Attachments
(3 files)
In bug 1306650 :froydnj added the task for building 32-bit clang-cl. This one is for 64-bit, because ASan needs 64-bit runtime binary for 64-bit Firefox.
![]() |
||
Comment 2•9 years ago
|
||
So our current clang-cl build task builds a 32-bit clang-cl, along with 32-bit runtime libraries. We really want a 64-bit clang-cl, with 32-bit and 64-bit runtime libraries, because our build machines are (AIUI) 64-bit machines anyway.
I *think* what needs to be done for this bug is to adjust the paths and whatnot in build-clang-windows.sh to use the 64-bit cl.exe etc. rather than the 32-bit cl.exe for building, and possibly adjust how clang-cl itself gets built. Is that right, dmajor? How have you been building 64-bit clang-cl?
I can do this, or Ting-Yu can do this, I think.
Flags: needinfo?(dmajor)
Yeah, I think it's just a matter of having the right compiler in your path. The only difference in my local workflow is whether I run cmake/ninja from the "VS2015 x64 Native Tools Command Prompt" or the x86 one.
Flags: needinfo?(dmajor)
One tricky thing is that even though 64-bit clang-cl can build for either target (since you can do -m32), I still end up building a separate 32-bit clang-cl just for its asan libaries. I don't know if there's an easier way around that.
Assignee | ||
Comment 6•9 years ago
|
||
The other platforms runs static analysis and ASan with only 64 bit version, I am not sure why but do we need both 32-bit and 64-bit for Windows?
![]() |
||
Comment 7•9 years ago
|
||
(In reply to Ting-Yu Chou [:ting] from comment #6)
> The other platforms runs static analysis and ASan with only 64 bit version,
> I am not sure why but do we need both 32-bit and 64-bit for Windows?
I would say because our other major platforms (Linux/OSX) are both 64-bit by default, so we focus on 64-bit ports there. Whereas 32-bit Windows is our primary Windows platform, so we'd want 32-bit static analysis. But we're also going to be shipping an official 64-bit Windows port soonish, so we'd want to analyze that as well.
That being said, if there a lot of overlap between our 32-bit and 64-bit Windows code, we could probably get away with doing one or the other. I don't know how much of our Windows code is specific to one architecture, though.
If we want to have Windows ASan be a full-fledged entry on treeherder running through the complete set of mochitests etc., then I suspect it's going to have to be 64-bit. We would OOM left and right on 32-bit builds.
![]() |
||
Comment 9•9 years ago
|
||
(In reply to David Major [:dmajor] from comment #8)
> If we want to have Windows ASan be a full-fledged entry on treeherder
> running through the complete set of mochitests etc., then I suspect it's
> going to have to be 64-bit. We would OOM left and right on 32-bit builds.
Indeed. But mochitests are a ways off at this point, AIUI. At least, talking with folks on the releng side indicated that running a couple of one-off Windows builds on each push is no big deal. Running the full test suite, under ASan or otherwise, is a different kettle of fish.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 13•9 years ago
|
||
mozreview-review |
Comment on attachment 8810710 [details]
Bug 1316537 part 1 - Add 64-bit clang-cl toolchain build.
https://reviewboard.mozilla.org/r/93002/#review93128
Attachment #8810710 -
Flags: review?(dustin) → review+
![]() |
||
Comment 14•9 years ago
|
||
(In reply to David Major [:dmajor] from comment #4)
> One tricky thing is that even though 64-bit clang-cl can build for either
> target (since you can do -m32), I still end up building a separate 32-bit
> clang-cl just for its asan libaries. I don't know if there's an easier way
> around that.
Does the 64-bit clang-cl not build any 32-bit runtime libraries, or does it just not build the ASan ones?
Given the problems with 32-bit ASan test running, I'm not sure if we care about 32-bit ASan builds...fuzzers might, though?
Flags: needinfo?(dmajor)
Comment 15•9 years ago
|
||
mozreview-review |
Comment on attachment 8810711 [details]
Bug 1316537 part 2 - Update 64-bit clang build configuration for the changes in bug 1306650.
https://reviewboard.mozilla.org/r/93004/#review93200
Attachment #8810711 -
Flags: review?(ehsan) → review+
Comment 16•9 years ago
|
||
mozreview-review |
Comment on attachment 8810712 [details]
Bug 1316537 part 3 - Fix tar package path conversion for local build.
https://reviewboard.mozilla.org/r/93006/#review93202
Attachment #8810712 -
Flags: review?(ehsan) → review+
![]() |
||
Comment 17•9 years ago
|
||
> Does the 64-bit clang-cl not build any 32-bit runtime libraries, or does it
> just not build the ASan ones?
afaik it doesn't build any 32-bit libraries.
Flags: needinfo?(dmajor)
Comment 18•9 years ago
|
||
Pushed by tchou@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f12149f08204
part 1 - Add 64-bit clang-cl toolchain build. r=dustin
https://hg.mozilla.org/integration/autoland/rev/bb4a098b3001
part 2 - Update 64-bit clang build configuration for the changes in bug 1306650. r=Ehsan
https://hg.mozilla.org/integration/autoland/rev/0e6e3d0eec49
part 3 - Fix tar package path conversion for local build. r=Ehsan
Assignee | ||
Comment 19•9 years ago
|
||
I'll check to make sure the task does work after landing.
Keywords: leave-open
Comment 20•9 years ago
|
||
bugherder |
Assignee | ||
Comment 21•9 years ago
|
||
Confirmed it works.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c76375b44f68a249df35902f9684db00b77d5be7
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Comment 22•7 years ago
|
||
Removing leave-open keyword from resolved bugs, per :sylvestre.
Keywords: leave-open
Updated•7 years ago
|
Product: TaskCluster → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•