Open Bug 1601903 Opened 7 months ago Updated 19 days ago

current development tree no longer builds with GCC and PGO

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

UNCONFIRMED

People

(Reporter: jh, Unassigned)

References

(Depends on 1 open bug)

Details

(Keywords: in-triage)

Attachments

(1 obsolete file)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:70.0) Gecko/20100101 Firefox/70.0

Steps to reproduce:

I built revision 505761:10160518ddc8 with GCC 9 and PGO.

Actual results:

It fials to find profdata and fails after porfiling. This is because GCC uses different format (gcda). This seems to be due to fact that build machinery was changed to keep both instrumented and non-instrumented tree.

I suppose Makefiles needs minor update to get gcda located correctly via -fprofile-generate=<dir> -fprofile-use=<dir> and to look for files in that directory.

Expected results:

It should have built

Depends on: build-gcc-9

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Component: Untriaged → General
Product: Firefox → Firefox Build System
Assignee: nobody → mshal

Now that the instrumented build is in a separated objdir from the
profile-use build, we need to point gcc to the correct directory to find
the gcda files. This is most easily done with a variable that gets
expanded by make to point to the instrumented objdir during the
profile-use build. Additionally, we have a separate cleanup routine in
profileserver.py to remove old *.gcda files.

Jan, can you try the attached patch please and let me know if you run into any issues? One odd thing I saw is way more -Wcoverage-mismatch errors during the build. I'm not sure yet why that is, but I'm still looking into it.

Flags: needinfo?(jh)

I am runing some tests on older tree but will try it soon.
-Wcoverage-mismatch warnings are usually caused by fact that profile collection was having race conditions. So reasonable quantity of those are expected (that is why we have -fprofile-correction). I will take a look if they got any worse than before.

I apologize for taking a while. I now have setup with top of tree and can do testing easily. However the approach with -fprofile-generate=<dir> -fprofile-use=<samedir> does not work with GCC9+. GCC8 used relative paths to name files in the profile directory and that was updated to use absolute paths in funny # mangling in GCC9+. This means that the -fprofile-feedback=<samedir> compilation is not able to find the gcda files produced at runtime because the absolute path of object file produced is different.

I will implement flag in GCC, but until that happens I think it is best to use -fprofile-generate and -fprofile-use and before starting -fprofile-use just copy gcda files to new location. Somehting like

cd <instrumentbuilddir>
tar cf <profdata> `find . -name "*.gcda"'
cd <feedbackbuildir>
tar xf <profdata>

Sorry for suggesting a wrong direction originally.

Flags: needinfo?(jh)

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:mshal, could you have a look please?
For more information, please visit auto_nag documentation.

Flags: needinfo?(mshal)
Attachment #9115637 - Attachment is obsolete: true

I abandoned the rev in phabricator because the -fprofile-use=<instrumented-dir> doesn't work as advertised. I also tried -fprofile-generate=<use-dir>, as well as the suggestion in #c5 to copy the *.gcda files in the instrumented build directory to the profile-use build directory. However, in all cases the profile-use build generates excessive -Wcoverage-mismatch warnings as compared to before the 3-tier local PGO changes. For example, compiling xpcom/threads/Unified_cpp_xpcom_threads0.o produces 337 warnings, while the original 1-tier gcc PGO build doesn't produce any for that file.

Jan, do you have any other suggestions to try? I'm unsure how to proceed.

Flags: needinfo?(mshal)
Assignee: mshal → nobody

I managed to do the PGO GCC build and seems to be functional despite the -Wcoverage-mismatch flood.

You need to log in before you can comment on or make changes to this bug.