This is an archive of the discontinued LLVM Phabricator instance.

Enable machine code verification with EXPENSIVE_CHECKS.
AbandonedPublic

Authored by fhahn on Mar 5 2017, 12:08 PM.

Details

Reviewers
RKSimon
davide
Summary

Currently quite a few tests fail with this patch. I've added a ticket to track the failures: https://bugs.llvm.org/show_bug.cgi?id=32146 . We may want to hold of enabling -verify-machineinstr until all the failures are resolved.

Diff Detail

Event Timeline

fhahn created this revision.Mar 5 2017, 12:08 PM
MatzeB added a subscriber: MatzeB.Mar 17 2017, 3:21 PM

Maybe we can add some sort of blacklisting mechanism, because it will require a lot of work to get all the targets fixed?

fhahn added a comment.Mar 22 2017, 3:29 AM

Are you thinking about something like XFAIL-EXPENSIVE?

RKSimon edited edge metadata.Mar 22 2017, 4:32 AM
RKSimon added a subscriber: qcolombet.

If at all possible I'd prefer to get some triage done on a majority of the bugs first before avoiding the problem in this way - my concern is that XFAIL tagging will be used just to ignore lurking problems that will bite back later on.

@qcolombet 's analyses in PR27481 covers quite a few cases and I think similar fixes should cover many of the remaining x86 failures, and I wouldn't be surprised if other targets are very similar.

Are you thinking about something like XFAIL-EXPENSIVE?

Well intuitively I would expect this to be a bigger issue, mostly because you have to get target maintainers on board to fix their targets.
Because of that I was thinking more in the lines of adding a bool TargetSubtargetInfo::isMachineVerifierClean() and adding a return false implementation to all the broken targets. That way we could fix the targets one by one.

I just proposed a "pragmatic" version of this: https://reviews.llvm.org/D33696

fhahn abandoned this revision.May 31 2017, 8:54 AM

I think we should go with @MatzeB 's patch https://reviews.llvm.org/D33696