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

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/test/CodeGen/X86/hhvm-cc.ll:106
 ; CHECK-NEXT:   {{^#.*}}
+; CHECK-NEXT:   pushq %r12
+; CHECK-NEXT:   pushq %rax
----------------
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.


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

https://reviews.llvm.org/D108731