Closed Bug 542504 Opened 11 years ago Closed 10 years ago

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


(Firefox Build System :: General, defect)

Windows 7
Not set


(Not tracked)



(Reporter: ted, Assigned: ted)




(1 file)

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.
Probably not, but it's probably simpler just to do it the right way, which means just adding a little bit of logic here:

(Trying to suppress it would probably be more of a PITA, honestly.)
Blocks: 581738
Assignee: nobody → ted.mielczarek
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+
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+
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.
Pushed to m-c:
Closed: 10 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.

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