Page MenuHomePhabricator

[MachineVerify] Verify property of atomic G_LOAD/STORE variants
AbandonedPublic

Authored by reames on Oct 3 2019, 1:11 PM.

Details

Summary

More or less directly copied from IR/Verifier.cpp.

Diff Detail

Event Timeline

reames created this revision.Oct 3 2019, 1:11 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 3 2019, 1:11 PM
arsenm added a comment.Oct 3 2019, 1:46 PM

Should have test in test/MachineVerifier

lib/CodeGen/MachineVerifier.cpp
1036–1037

IMO we should remove alignment 0 from all points in the IR and MIR

arsenm added inline comments.Oct 3 2019, 2:18 PM
lib/CodeGen/MachineVerifier.cpp
1036–1037

On second thought, I'm not sure this is necessary. The MIRParser won't ever produce a 0 base alignment MMO. This can probably be replaced with an assert in the MMO constructor

reames planned changes to this revision.Oct 3 2019, 3:16 PM
reames marked an inline comment as done.

Pending refresh w/suggested changes.

lib/CodeGen/MachineVerifier.cpp
1036–1037

This is a good idea. I will do so.

arsenm added inline comments.Oct 3 2019, 4:54 PM
lib/CodeGen/MachineVerifier.cpp
1036–1037

I'm not sure the MIParser rejects 0, it just doesn't default to 0. It would be a good idea to make sure that happens also

reames abandoned this revision.Oct 19 2019, 2:16 PM

It turns out the MachineMemOperand constructor already asserts on a zero alignment. The vector check is potentially useful, but given this doesn't cover SelectionDAG anyway (where I'm currently focused), I'm deferring it until a future point.