SIMPLE_PROGRAMS (and CPP_UNIT_TESTS) don't get re-linked in a PGO build

RESOLVED FIXED

Status

Firefox Build System
General
RESOLVED FIXED
8 years ago
2 months ago

People

(Reporter: ted, Assigned: ted)

Tracking

Trunk
x86
Windows 7
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

8 years ago
I hit this trying to run tests from a downloaded unit test package on Windows without having VC8 installed. One of the test programs (WriteArgument.exe) fails popping up a dialog telling me I don't have PGORT80.DLL installed. Clearly that didn't get re-linked during the second pass of the PGO build, or it wouldn't be linked to that DLL.
Blocks: 578448
Do we even need to PGO SIMPLE_PROGRAMS?
Seems like we should just suppress PGO completely for SIMPLE_PROGRAMS.
(Assignee)

Comment 3

8 years ago
Probably not, but it's probably simpler just to do it the right way, which means just adding a little bit of logic here:
http://mxr.mozilla.org/mozilla-central/source/config/rules.mk#935

(Trying to suppress it would probably be more of a PITA, honestly.)
Blocks: 581738
(Assignee)

Updated

8 years ago
Assignee: nobody → ted.mielczarek
(Assignee)

Comment 4

8 years ago
Created attachment 460250 [details] [diff] [review]
Don't link SIMPLE_PROGRAMS with PGO flags

Ok, I lied. Turns out it was easy enough to skip linking SIMPLE_PROGRAMS with the PGO flags. I didn't do a full PGO build locally, but I did the first half, which is where they'd normally wind up linked with that PGORT DLL, and verified (using Dependency Walker) that a sampling of the test programs are not linked to it.

We'll probably need to clobber the Win32 opt builders after landing this to make sure that all the test programs get rebuilt.
Attachment #460250 - Flags: review?(mitchell.field)
This patch means that if I set SIMPLE_PROGRAMS in a directory that turns off PGO for anything in that directory (in other words, if I added SIMPLE_PROGRAMS to toolkit/library I basically turned off PGO).  That's acceptable IMHO (people shouldn't be mixing test/helper code into important directories) but probably should be documented somewhere.
Attachment #460250 - Flags: review?(mitchell.field) → review+
(Assignee)

Comment 6

8 years ago
Comment on attachment 460250 [details] [diff] [review]
Don't link SIMPLE_PROGRAMS with PGO flags

This will fix some of our unit tests on Windows 7.
Attachment #460250 - Flags: approval2.0?
Attachment #460250 - Flags: approval2.0? → approval2.0+
(Assignee)

Comment 7

8 years ago
Kyle: I'm not particularly concerned about that. We don't actually do that anywhere, AFAICT, mostly because our build system makes it a pain to build more than one thing per-directory.
(Assignee)

Comment 8

8 years ago
Pushed to m-c:
http://hg.mozilla.org/mozilla-central/rev/56d3a4ea8218
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(In reply to comment #4)
> We'll probably need to clobber the Win32 opt builders after landing this to
> make sure that all the test programs get rebuilt.

Clobbering.

Updated

8 years ago
Depends on: 602245

Updated

2 months ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.