[PATCH] D68267: [MBB LiveIn lists, MachineVerifier, SystemZ] New method isLiveOut() and mverifier improvement.

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added a comment.

In D68267#1733661 <https://reviews.llvm.org/D68267#1733661>, @jonpa wrote:

> - CodeGen/X86/copy-eflags.ll
>
>   "X86 EFLAGS copy lowering" transforms the def-use lists of $eflags (to local def-uses) without updating the livein lists of successor blocks:
>
>
>
>   bb.1.bb1:
>     ...
>     $eflags = COPY %16:gr32
>     JCC_1 %bb.3, 12, implicit $eflags
>  
>   bb.2.bb1:
>   ; predecessors: %bb.1
>     successors: %bb.3(0x80000000); %bb.3(100.00%)
>     liveins: $eflags
>  
>   bb.3.bb1:
>   ; predecessors: %bb.1, %bb.2
>     successors: %bb.4(0x40000000), %bb.5(0x40000000); %bb.4(50.00%), %bb.5(50.00%)
>     liveins: $eflags
>     %2:gr8 = PHI %8:gr8, %bb.2, %0:gr8, %bb.1
>     MOV8mr %9:gr32, 1, $noreg, 0, $noreg, %2:gr8 :: (volatile store 1 into %ir.ptr1)
>     %20:gr32 = MOV32rm %10:gr32, 1, $noreg, 0, $noreg :: (volatile load 4 from %ir.ptr2)
>     JCC_1 %bb.5, 12, implicit $eflags
>  
>   =>
>  
>   After X86 EFLAGS copy lowering:
>  
>   bb.1.bb1:
>     TEST8rr %23:gr8, %23:gr8, implicit-def $eflags
>     JCC_1 %bb.3, 5, implicit killed $eflags
>  
>   bb.2.bb1:
>   ; predecessors: %bb.1
>     successors: %bb.3(0x80000000); %bb.3(100.00%)
>     liveins: $eflags
>  
>   bb.3.bb1:
>   ; predecessors: %bb.1, %bb.2
>     successors: %bb.4(0x40000000), %bb.5(0x40000000); %bb.4(50.00%), %bb.5(50.00%)
>     liveins: $eflags
>     %2:gr8 = PHI %8:gr8, %bb.2, %0:gr8, %bb.1
>     MOV8mr %9:gr32, 1, $noreg, 0, $noreg, %2:gr8 :: (volatile store 1 into %ir.ptr1)
>     %20:gr32 = MOV32rm %10:gr32, 1, $noreg, 0, $noreg :: (volatile load 4 from %ir.ptr2)
>     TEST8rr %23:gr8, %23:gr8, implicit-def $eflags
>     JCC_1 %bb.5, 5, implicit killed $eflags
>
>
> It seems that since X86FlagsCopyLoweringPass::rewriteCondJmp() does JmpI.findRegisterUseOperand(X86::EFLAGS)->setIsKill(true), it should be safe to assume that EFLAGS is not live out. In that case it should also follow that any successor block will not use a live-in value of EFLAGS. So any successor block should have EFLAGS removed of the live-in list, right?
>
> I am not sure what is the right way to clear EFLAGS of the successor blocks live-in lists. Should it be done in rewriteCondJmp() only or are there other places also that this should be done?


I haven't touched this is sometime. The thing I would worry about is whether *all* of the edges into the successor have been rewritten such that EFLAGS no longer needs to live-in. I don't recall whether the code ensures that is the case in a clear point, but if it does, that is the place to update the live-in list. (I suspect it does somewhere, and it is just finding it in order to do the update correctly... my guess is that it'll be at the end because we're operating edge-wise and may not have finished rewriting edges into the successor... but that's just a guess.)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68267/new/

https://reviews.llvm.org/D68267