Page MenuHomePhabricator

Don't add repeats of llvm.ident list when linking
Needs ReviewPublic

Authored by arsenm on May 24 2016, 10:56 AM.

Details

Summary

Fixes spam in the common case when linking together
many modules produced by the same compiler version.

Diff Detail

Event Timeline

arsenm updated this revision to Diff 58273.May 24 2016, 10:56 AM
arsenm retitled this revision from to Don't add repeats of llvm.ident list when linking.
arsenm updated this object.
arsenm added a subscriber: llvm-commits.
rafael edited reviewers, added: dexonsmith; removed: rafael.Aug 24 2016, 2:07 PM

Is there a reason not to dedup all metadata nodes?

rivanvx added a subscriber: rivanvx.Sep 7 2016, 2:21 PM

It’s unfortunate that this will be quadratic in the number of modules. Is there a way we could improve that?

It’s unfortunate that this will be quadratic in the number of modules. Is there a way we could improve that?

Well the linker API only exposes linking one module into one other at at time, so I don't know. Currently we have the AMDGPUUnifyMetadata pass as a workaround which cleans these up in a pass over the fully linked module, so that avoids revisiting for each module but it would make more sense if the linker dealt with this.

It’s unfortunate that this will be quadratic in the number of modules. Is there a way we could improve that?

Well the linker API only exposes linking one module into one other at at time, so I don't know. Currently we have the AMDGPUUnifyMetadata pass as a workaround which cleans these up in a pass over the fully linked module, so that avoids revisiting for each module but it would make more sense if the linker dealt with this.

I wonder whether it would be generally useful to support SetVectors in named metadata nodes. I'm not sure what the textual IR syntax would be, but I believe debug info would use it too.

lib/Linker/IRMover.cpp
1001–1005

Do you need to reach inside to the strings? Why not just compare the nodes themselves?

It’s unfortunate that this will be quadratic in the number of modules. Is there a way we could improve that?

Well the linker API only exposes linking one module into one other at at time, so I don't know. Currently we have the AMDGPUUnifyMetadata pass as a workaround which cleans these up in a pass over the fully linked module, so that avoids revisiting for each module but it would make more sense if the linker dealt with this.

I wonder whether it would be generally useful to support SetVectors in named metadata nodes. I'm not sure what the textual IR syntax would be, but I believe debug info would use it too.

Something more general would be better. The custom pass currently handles 6 different items