Page MenuHomePhabricator

[compiler-rt] Remove copy of InstrProfData.inc
Needs ReviewPublic

Authored by kpdev42 on Aug 31 2020, 12:47 PM.

Details

Summary

There are two identical files with instrprofdata description: one in LLVM (so called "master" file), another one in compiler-rt.
These files should be identical, but from time to time they slightly differs (e.g. now since https://reviews.llvm.org/D84261).
I propose to remove copy of InstrProfData.inc from compiler-rt. If there are some concerns about it, please let me know (I asked about it in mailing list, but unfortunately didn't receive an answer http://llvm.1065342.n5.nabble.com/llvm-dev-compiler-rt-InstrProfData-inc-duplicate-td137260.html)

Diff Detail

Event Timeline

kpdev42 created this revision.Aug 31 2020, 12:47 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptAug 31 2020, 12:47 PM
Herald added subscribers: llvm-commits, Restricted Project, aaron.ballman and 2 others. · View Herald Transcript
kpdev42 requested review of this revision.Aug 31 2020, 12:47 PM

Gentle ping about removing this unnecessary copy. Please share your thoughts here about it @yamauchi @davidxl @rnk

Change in this direction is welcome. Added vsk and beanz to comment on the implication on runtime builds.

I would love to have a single source of truth, but this basically makes it
impossible to just build compiler-rt by itself, no?

That path is also “very non portable”, given that our cmake build system
can find/be directed to the source for the projects in different places.

Thank you,
Filipe

I would love to have a single source of truth, but this basically makes it
impossible to just build compiler-rt by itself, no?

That path is also “very non portable”, given that our cmake build system
can find/be directed to the source for the projects in different places.

Thank you,
Filipe

Thank you for the feedback.
If it is a important case when compiler-rt should be build separately from llvm-project,
then maybe I'll add a motivation for it to file description (like compiler-rt should have a possibility to be built out of tree)
and add a test to compiler-rt which will check these files to be identical?

Or it is better to leave everything as it is? :)

yamauchi added a comment.EditedSep 14 2020, 9:13 AM

I'm not familiar with the history of this file well, but I had the same impression: it'd be nice if there's a single copy but since compiler-rt is set up to build without depending on llvm/, it's in the way it is currently.

Adding a test to make sure they are in sync will be useful.