[PATCH] D84479: [X86] Detect if EFLAGs is live across XBEGIN pseudo instruction. Add it as livein to the basic blocks created when expanding the pseudo

[Craig Topper via Phabricator via llvm-commits]

craig.topper added a comment.

In D84479#2173864 <https://reviews.llvm.org/D84479#2173864>, @ivanbaev wrote:

> Hi Craig, thank you for working on this bug.
>
> 1. *** IR Dump Before Finalize ISel and expand pseudo-instructions ***:
> 2. Machine code for function wobble.12: IsSSA, TracksLiveness
>
>   bb.0.bb107: successors: %bb.1(0x40000000), %bb.2(0x40000000); %bb.1(50.00%), %bb.2(50.00%)
>
>   %0:gr32 = MOV32rm %fixed-stack.0, 1, $noreg, 0, $noreg :: (load 4 from %fixed-stack.0, align 16) %1:gr32 = SUB32ri8 %0:gr32(tied-def 0), 1, implicit-def $eflags %2:gr32 = XBEGIN JCC_1 %bb.2, 5, implicit $eflags JMP_1 %bb.1 ...
>
>   ----------------------------------------------------------------------------- Given the IR above for the test, an alternative liveness check for eflags is to iterate backward. Look for a def of eflag, and if we reach the start of the basic block: check BB->isLiveIn(X86::EFLAGS)) and avoid iterating over successors of BB as in the proposed fix.


We'd have to also use the kill flag for any users we find before we find the def? We shouldn't make it livein if it was already killed.

> As optimization add EFLAGS only to sinkMBB
>  sinkMBB->addLiveIn(X86::EFLAGS);

Can we have it live in in sinkMBB without it being livein in the predecessors in sinkMBB?

> The proposed fix is good too. It just seems to me that connecting a def to its uses is more natural.




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

https://reviews.llvm.org/D84479