Page MenuHomePhabricator

[PowerPC] Fix the assert of ISD::SIGN_EXTEND_INREG when type is v2i16 and v2i8
ClosedPublic

Authored by wuzish on Sep 24 2018, 8:59 PM.

Details

Summary

For ISD::SIGN_EXTEND_INREG operation of v2i16 and v2i8 types will cause assert because they are registered as custom operation. So that the type legalization phase will enter the custom hook, which do not handle ISD::SIGN_EXTEND_INREG operation and fall throw into unreachable assert.

For ISD::SIGN_EXTEND_INREG operation of vector type can be easily expanded registered as default legalize operation behavior. I think this is a clean-up work after the patch https://reviews.llvm.org/D20443.

Diff Detail

Repository
rL LLVM

Event Timeline

wuzish created this revision.Sep 24 2018, 8:59 PM
wuzish edited the summary of this revision. (Show Details)Sep 24 2018, 9:01 PM
nemanjai accepted this revision.Sep 25 2018, 6:51 AM

LGTM. Thanks for fixing this.

llvm/test/CodeGen/PowerPC/2018-09-19-sextinreg-vector-crash.ll
3 ↗(On Diff #166800)

That's fine. Please add just a couple of basic checks for the code we generate. It doesn't have to be elaborate at all, just check for a couple of key instructions.

This revision is now accepted and ready to land.Sep 25 2018, 6:51 AM
wuzish updated this revision to Diff 167041.Sep 25 2018, 9:28 PM

Add some code sequence check.

wuzish marked an inline comment as done.Sep 25 2018, 9:28 PM

Can we get this committed? This is good code cleanup that also fixes crashes (albeit ones that nobody has come across in the field).
The subsequent patch is controversial and we can deal with it separately, but this portion is non-controversial and should go upstream.

@wuzish You could apply the commit access from community. I will commit the patch for you.

wuzish added a comment.Oct 9 2018, 7:23 PM

@wuzish You could apply the commit access from community. I will commit the patch for you.

Thank you a lot. I will try.

This revision was automatically updated to reflect the committed changes.