This is an archive of the discontinued LLVM Phabricator instance.

[TwoAddressInstruction] Contrain RegClass when processing a statepoint
ClosedPublic

Authored by danilaml on Dec 26 2022, 6:04 AM.

Details

Summary

This transformation could've triggered a verifier assert if RegA and RegB
were of different reg classes. Fix this by constraining as the comment
for replaceRegWith suggests.

Diff Detail

Event Timeline

danilaml created this revision.Dec 26 2022, 6:04 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 26 2022, 6:04 AM
danilaml requested review of this revision.Dec 26 2022, 6:04 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 26 2022, 6:04 AM
dantrushin accepted this revision.Dec 26 2022, 6:29 AM

LGTM, but clean test up before committing.

llvm/test/CodeGen/AArch64/statepoint-twoaddr.mir
38

Please remove redundant constant arguments.
You don't need 47 constant deopt arg in this test. I would guess you don't need those undef call arguments(or even _any_ of them?).
At a glance only %0 is essential for the test.

Just in case, format of statepoint MI is as follows:

///   <id>, <num patch bytes>, <num call arguments>, <call target>,
///   [call arguments...],
///   <StackMaps::ConstantOp>, <calling convention>,
///   <StackMaps::ConstantOp>, <statepoint flags>,
///   <StackMaps::ConstantOp>, <num deopt args>, [deopt args...],
///   <StackMaps::ConstantOp>, <num gc pointers>, [gc pointers...],
///   <StackMaps::ConstantOp>, <num gc allocas>,  [gc allocas...]
///   <num entries in gc map>, <base/derived indices...>

Where StackMaps::ConstantOp is literal '2' above.

This revision is now accepted and ready to land.Dec 26 2022, 6:29 AM
danilaml updated this revision to Diff 485303.Dec 26 2022, 7:49 AM

Simplified the test

danilaml marked an inline comment as done.Dec 26 2022, 7:50 AM
This revision was landed with ongoing or failed builds.Dec 26 2022, 8:01 AM
This revision was automatically updated to reflect the committed changes.