Page MenuHomePhabricator

[Bitcode] Drop invalid branch_weight in BitcodeReader
ClosedPublic

Authored by steven_wu on Mon, Jul 13, 10:50 AM.

Details

Summary

If bitcode reader gets an invalid branch weight, drop that from the
inputs. This allows us to read the broken modules we generated before
the verifier was able to catch this.

rdar://64870641

Diff Detail

Event Timeline

steven_wu created this revision.Mon, Jul 13, 10:50 AM
Herald added a project: Restricted Project. · View Herald TranscriptMon, Jul 13, 10:50 AM

So this is a bitcode "upgrade" of sorts. An alternative would be to implement this in Verifier and strip invalid metadata similar to how invalid debug info is stripped, but treating this as an upgrade in the bitcode reader is probably fine too.

llvm/test/Bitcode/branch-weight.ll
15

Can you add one positive CHECK, too, to make sure this doesn't just print nothing at all?

Address review feedback and also add a testcase to make sure good branch
weights are not stripped.

I don't think the debug info approach is good here since it is a lot of
information to pass through to only strip the bad branch weights. Also
not sure if it is really a good idea to run verifier in upgrader.

aprantl added inline comments.Tue, Jul 21, 1:44 PM
llvm/lib/Bitcode/Reader/BitcodeReader.cpp
5397

Perhaps even more explicit:
// "Upgrade" older incorrect branch weights by dropping them.

5400

Are the || operators correct here?

I feel like this should be

MD->getOperand(0) != nullptr && isa<MDString>(MD->getOperand(0) ?

5405

if (!ProfName.equals("branch_weights"))

continue;
5420

doesn't

steven_wu updated this revision to Diff 279960.Wed, Jul 22, 3:38 PM

Address review feedback

steven_wu marked 4 inline comments as done.Wed, Jul 22, 3:39 PM
aprantl accepted this revision.Wed, Jul 22, 5:23 PM
This revision is now accepted and ready to land.Wed, Jul 22, 5:23 PM
This revision was automatically updated to reflect the committed changes.