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.
Paths
| Differential D38001
[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
Event Timelinecraig.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 Comment Actions 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? Comment Actions 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? Comment Actions 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 Closed by commit rL314152: [X86] Don't select anyext GR32->GR64 to SUBREG_TO_REG. Use INSERT_SUBREG… (authored by ctopper). · Explain WhySep 25 2017, 2:16 PM This revision was automatically updated to reflect the committed changes.
Revision Contents
Diff 116613 llvm/trunk/lib/Target/X86/X86InstrCompiler.td
llvm/trunk/test/CodeGen/X86/vector-shuffle-variable-128.ll
llvm/trunk/test/CodeGen/X86/vector-shuffle-variable-256.ll
|