Page MenuHomePhabricator

[NFC][AVR] Remove unused isMachineVerifierClean()
ClosedPublic

Authored by xgupta on Aug 10 2021, 11:55 AM.

Details

Summary

r304320 enabled the machine verifier by default with
EXPENSIVE_CHECKS. Mistakenly AVR was in the one of the targets
excluded from this by overriding TargetMachine::isMachineVerifierClean() to
return false in D33696.

Removing the override as the target passes all lit tests without machine
verifier failure.

Diff Detail

Event Timeline

xgupta created this revision.Aug 10 2021, 11:55 AM
xgupta requested review of this revision.Aug 10 2021, 11:55 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 10 2021, 11:55 AM
mhjacobson accepted this revision.Aug 12 2021, 11:02 AM

Do we happen to know what change(s) made this pass work correctly for AVR?

This revision is now accepted and ready to land.Aug 12 2021, 11:02 AM

IIUC this was added defensively to follow other targets. Seems it was not needed for AVR. And also please commit on my behalf if you are ok with changes, git attribution - "Shivam Gupta <xgupta@aol.com>".

benshi001 accepted this revision.Aug 12 2021, 7:50 PM
This revision was automatically updated to reflect the committed changes.
aykevl added a reviewer: aykevl.EditedJan 6 2022, 5:33 AM
aykevl added a subscriber: aykevl.

I actually made several patches to the AVR backend to make it machine verifier clean, so it was certainly not incorrectly disabled. Here are the patches: D96969, D97127, D97172, D97131, D97159. This did in fact find a few actual bugs, so I'm very happy with the work on the machine verifier!

xgupta added a comment.Jan 6 2022, 5:39 AM

Oh, Thank you for your efforts @aykevl!