Page MenuHomePhabricator

IR Linking: Support merging Warning+Max module metadata flags
ClosedPublic

Authored by dblaikie on Feb 7 2020, 2:41 PM.

Details

Summary

Debug Info Version was changed to use "Max" instead of "Warning" per the
original design intent - but this maxes old/new IR unlinkable, since
mismatched merge styles are a linking failure.

It seems possible/maybe reasonable to actually support the combination
of these two flags: Warn, but then use the maximum value rather than the
first value/earlier module's value.

To fix: https://bugs.llvm.org/show_bug.cgi?id=44784

Diff Detail

Event Timeline

dblaikie created this revision.Feb 7 2020, 2:41 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 7 2020, 2:41 PM
dblaikie edited the summary of this revision. (Show Details)
dmajor added a subscriber: dmajor.Feb 7 2020, 3:02 PM
tejohnson accepted this revision.Feb 7 2020, 3:16 PM

LGTM with a couple minor suggestions below.
I think this makes sense.

llvm/lib/Linker/IRMover.cpp
1377

A comment about why this isn't in the switch statement would be helpful, along with a note about how the new Flag is formed.

Also, it might be clearer to put this before the switch, after the new warning handling, with a continue to skip the then unnecessary switch (it consolidates the special case handling for Warning+Max).

This revision is now accepted and ready to land.Feb 7 2020, 3:16 PM
dblaikie marked an inline comment as done.Feb 7 2020, 4:30 PM
dblaikie added inline comments.
llvm/lib/Linker/IRMover.cpp
1377

Makes sense, thanks! Did those.

This revision was automatically updated to reflect the committed changes.