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

[Andy Kaylor via Phabricator via llvm-commits]

andrew.w.kaylor added a comment.

In https://reviews.llvm.org/D40524#940330, @a.elovikov wrote:

> I'm not sure if the following is possible/legal but it fails even with this patch:
>
>   body:             |
>     bb.0:
>       successors:
>       liveins: %ch, %bl
>  
>       %cl = MOV8rr %bl, implicit-def %cx, implicit killed %ch, implicit-def %eflags
>       ; CHECK-NOT: MOV32rr
>       RETQ %cx
>  
>
>
> If that's a valid testcase than "a little more checking to do." (isLive before this patch) has to be fixed. If it's invalid - we'd need an assert for that too (or some mir-verify?). However, that should not be a part of this change, probably. Hence, just asking what's your opinion about that testcase.


Hi Andrei,

I apologize for the long delay in responding. I've been distracted with other things and haven't committed this change yet.

I'm not terribly familiar with MIR as it appears in a test like this, but I can't see any reason that your proposed testcase would be invalid. I do think it's extremely unlikely to arise in actual generated Machine IR, since we hardly ever use the 8-bit high registers. What I'd like to do is commit the change from this review as is, and file a new Bugzilla report to track the 8bit_hi issue.  Does that sound reasonable to you?


https://reviews.llvm.org/D40524