This is an archive of the discontinued LLVM Phabricator instance.

[AArch64][GlobalISel]: Fix a crash in GlobalIsel in dealing with 16bit uadd.with.overflow.
ClosedPublic

Authored by xiaoqing_wu on Dec 16 2019, 6:31 PM.

Details

Summary

AArch64 doesn't support uadd.with.overflow.i16 natively. This change adds a legalization rule to convert the 32bit add result to 16bit. This should fix PR43981.

Diff Detail

Event Timeline

xiaoqing_wu created this revision.Dec 16 2019, 6:31 PM
xiaoqing_wu retitled this revision from Fix a crash in global-isel in dealing with 16bit uadd.with.overflow. to [GlobalISel]: Fix a crash in GlobalIsel in dealing with 16bit uadd.with.overflow on AArch64.Dec 16 2019, 7:09 PM
xiaoqing_wu edited the summary of this revision. (Show Details)
xiaoqing_wu retitled this revision from [GlobalISel]: Fix a crash in GlobalIsel in dealing with 16bit uadd.with.overflow on AArch64 to [AArch64][GlobalISel]: Fix a crash in GlobalIsel in dealing with 16bit uadd.with.overflow..Dec 16 2019, 7:15 PM
xiaoqing_wu edited the summary of this revision. (Show Details)
paquette accepted this revision.Dec 17 2019, 9:26 AM

Thanks for fixing this!

LGTM with some minor nits on the test.

Also, can you rename the test to "legalize-uaddo.mir"? That way we can reuse it for any future legalization work we might have to do for G_UADDO.

llvm/test/CodeGen/AArch64/test-uadd16-with-overflow.mir
2 ↗(On Diff #234213)

I would add -verify-machineinstrs to the RUN line here.

9–20 ↗(On Diff #234213)

I'm pretty sure you can delete the registers, liveins, frameInfo, and machineFunctionInfo sections in this test. The legalizer doesn't use any of this information, and so it's not necessary to keep it.

This revision is now accepted and ready to land.Dec 17 2019, 9:26 AM

Updated the test case according to review comment.

Remove old test case

paquette accepted this revision.Dec 17 2019, 11:55 AM
This revision was automatically updated to reflect the committed changes.