[PATCH] D40524: Handle the case of live 16-bit subregisters in X86FixupBWInsts

Andy Kaylor via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 28 11:45:37 PST 2017


andrew.w.kaylor added inline comments.


================
Comment at: test/CodeGen/X86/fixup-bw-inst.mir:195
+
+    %r9b = MOV8rr undef %r10b, implicit-def %eflags
+    ; CHECK-NOT: MOV32rr
----------------
a.elovikov wrote:
> I'm surprised not to see "%R9D<imp-use,kill>, %R9D<imp-def>" from the Bugzilla here. IIIC the use below does not prevent anything because it might be use-undef so I'm not sure what exactly in this test prevents the transformation.
As the test is written, it hits the case where the super-register is detected as live after the instruction (because LivePhysRegs sees the use of r9w and so adds it to the live set) but we don't see either an implicit use or an implicit def of the register in the instruction, so we assume it must have been live before the instruction. I think that this will be true in actual Machine IR that has been produced normally. In this case, the MIR in the test is just incomplete. I suppose the use on line 197 isn't considered as "possibly undef" just because the MIR in the test doesn't say it is undef.

I'll update this to make the test look like the actual failure that was seen in the wild.

BTW, I did run this test without my patch and it fails (because r9d isn't live and we weren't checking r9w). The test also fails (even with my change) if I add 'implicit-def %r9d' but not 'implicit kill %r9d'.


https://reviews.llvm.org/D40524





More information about the llvm-commits mailing list