This is an archive of the discontinued LLVM Phabricator instance.

[X86] Don't select anyext GR32->GR64 to SUBREG_TO_REG. Use INSERT_SUBREG instead.
ClosedPublic

Authored by craig.topper on Sep 18 2017, 3:02 PM.

Details

Summary

As far as I know SUBREG_TO_REG is stating that the upper bits are 0. But if we are just converting the GR32 with no checks, then we have no reason to say the upper bits are 0.

Seems to me like we should be selecting this to INSERT_SUBREG.

Diff Detail

Repository
rL LLVM

Event Timeline

craig.topper created this revision.Sep 18 2017, 3:02 PM
craig.topper retitled this revision from [X86] Don't select any extend of anyext GR32->GR64 to SUBREG_TO_REG. Use INSERT_SUBREG instead. to [X86] Don't select anyext GR32->GR64 to SUBREG_TO_REG. Use INSERT_SUBREG instead..Sep 18 2017, 10:27 PM
DavidKreitzer edited edge metadata.Sep 19 2017, 5:25 AM

This change seems obviously correct. The diffs in the tests all look like tweaks in the register allocation behavior and aren't testing the behavior you are fixing. Is it possible to write a test that miscompiles due to the existing bug?

I havent' found any piece of code that analyzes SUB_REG_TO_REG that closely. Everything I found pretty much treats it much like a copy. Does anyone know of anything that cares today?

DavidKreitzer accepted this revision.Sep 25 2017, 1:53 PM

I'd recommend going ahead with this, Craig. Even if it isn't possible for this bug to trigger a failure today, it looks like a latent bug that should be fixed.

This revision is now accepted and ready to land.Sep 25 2017, 1:53 PM
This revision was automatically updated to reflect the committed changes.