This is an archive of the discontinued LLVM Phabricator instance.

IR: Disable verifier check for GlobalValues with private linkage named after a comdat for non-COFF.
ClosedPublic

Authored by pcc on Aug 2 2019, 5:33 PM.

Details

Diff Detail

Repository
rL LLVM

Event Timeline

pcc created this revision.Aug 2 2019, 5:33 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 2 2019, 5:33 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
pcc updated this revision to Diff 213152.Aug 2 2019, 5:34 PM
  • Remove decl
Harbormaster completed remote builds in B36057: Diff 213152.
rnk added a comment.Aug 5 2019, 11:57 AM

I haven't had time to really think about this, but I believe @majnemer felt it was important to add verifier checks like these. There's no reason we can't make this triple dependent, right? If there's no triple on the module, we can skip the check.

pcc added a comment.Aug 5 2019, 12:18 PM

Making it conditional on the triple is fine with me I suppose. It's unfortunate that we use the same IR for ELF and COFF here, maybe longer term we should move towards exclusively using !associated in COFF.

pcc updated this revision to Diff 213431.Aug 5 2019, 12:24 PM
  • Check the triple
pcc retitled this revision from IR: Remove verifier check for GlobalValues with private linkage named after a comdat. to IR: Disable verifier check for GlobalValues with private linkage named after a comdat for non-COFF..Aug 5 2019, 12:25 PM
pcc edited the summary of this revision. (Show Details)
rnk accepted this revision.Aug 6 2019, 11:45 AM

In the past, Rafael advocated for having more verifier check more things that are unrepresentable at an object file level. That's how we ended up with most of the checks in Verifier::visitGlobalAlias. I think this is consistent with that, even if we have to check which object file invariant we're enforcing.

lgtm

This revision is now accepted and ready to land.Aug 6 2019, 11:45 AM
This revision was automatically updated to reflect the committed changes.