[PATCH] D108731: [TwoAddressInstructionPass] Improve the SrcRegMap and DstRegMap computation

[Guozhi Wei via Phabricator via llvm-commits]

Carrot added inline comments.


================
Comment at: llvm/test/CodeGen/X86/hhvm-cc.ll:106
 ; CHECK-NEXT:   {{^#.*}}
+; CHECK-NEXT:   pushq %r12
+; CHECK-NEXT:   pushq %rax
----------------
craig.topper wrote:
> Carrot wrote:
> > craig.topper wrote:
> > > This looks worse
> > The code before TwoAddress pass is:
> > 
> > bb.0.entry:
> >   liveins: $rbx, $r12, $rbp
> >   %2:gr64 = COPY killed $rbp
> >   %1:gr64 = COPY killed $r12
> >   %0:gr64 = COPY killed $rbx
> >   ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
> >   %3:gr64 = MOV32ri64 42
> >   $rbx = COPY killed %0:gr64
> >   $r12 = COPY %1:gr64
> >   $rbp = COPY killed %2:gr64
> >   $r15 = COPY killed %3:gr64
> >   CALL64pcrel32 target-flags(x86-plt) @php_short, <regmask $r12 $r12b $r12bh $r12d $r12w $r12wh>, implicit $rsp, implicit $ssp, implicit killed $rbx, implicit killed $r12, implicit killed $rbp, implicit killed $r15, implicit-def $rsp, implicit-def $ssp, implicit-def dead $rbx, implicit-def $rbp
> >   ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp 
> >   %5:gr64 = COPY killed $rbp 
> >   %6:gr64 = ADD64rr killed %5:gr64(tied-def 0), killed %1:gr64, implicit-def dead $eflags
> >   $rbx = COPY killed %6:gr64
> >   RET 0, killed $rbx 
> > 
> > In function call to php_short, $r12 is clobbered, with my patch, the map SrcRegMap[%1] = $r12 is cleared, it causes the following ADD instruction commuted. And later different registers are passed to isProfitableToConv3Addr, so different result is returned, and different lea/add instruction is generated.
> > 
> > I'm also surprised by the RA result, the live range of %1 crosses the function call, $r12 is killed and clobbered by the function call, why does $r12 still allocated to %1? The direct usage of $r12 after the function call looks wrong to me.
> > 
> The regmask for the function says r12 is preserved. All other registers are clobbered.
Thanks for the explanation! Now I understand the problem is at

$r12 = COPY %1:gr64

Before this instruction %1 is already mapped to $r12, copy it back to $r12 should not invalidate the map entry SrcRegMap[%1] = $r12. I should make the check in function removeClobberedSrcRegMap.


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

https://reviews.llvm.org/D108731