[PATCH] D143425: Reland "[X86][ABI] Don't preserve return regs for preserve_all/preserve_most CCs""

[Anton Bikineev via Phabricator via llvm-commits]

AntonBikineev added a comment.

In D143425#4110057 <https://reviews.llvm.org/D143425#4110057>, @pengfei wrote:

> In D143425#4109985 <https://reviews.llvm.org/D143425#4109985>, @AntonBikineev wrote:
>
>> In D143425#4109955 <https://reviews.llvm.org/D143425#4109955>, @pengfei wrote:
>>
>>>> This covers the broken scenario.
>>>
>>> Can you explain it in more detail? I checked the test locally but found no difference in codegen with or without the new change.
>>
>> Did you include the original (broken) CL in the baseline? The CL was reverted, so you'd have to apply it to see the codegen diff. But the diff is basically this:
>
> No, I compared between commenting out `if (ShouldDisableArgRegs) {` and not, and didn't see any difference. Or I misunderstood something?

Indeed, the tests didn't cover this case. Thanks! I added new tests dynamic-regmask-preserve-*.ll to check exactly the preserved RegMask on the caller side. Now the difference between the broken commit is much clearer on the caller side. For example, commenting out `if (ShouldDisableArgRegs) {` would remove `rdi`, `rsi`, `rdx`, `rcx`, `r8` and their subregisters from the `CustomRegMask` in the call:

  CALL64pcrel32 @callee1, CustomRegMask($bh,$bl,$bp,$bph,$bpl,$bx,$ch,$cl,$cx,$dh,$di,$dih,$dil,$dl,$dx,$ebp,$ebx,$ecx,$edi,$edx,$esi,$hbp,$hbx,$hcx,$hdi,$hdx,$hsi,$rbp,$rbx,$rcx,$rdi,$rdx,$rsi,$si,$sih,$sil,$r8,$r9,$r10,$r12,$r13,$r14,$r15,$r8b,$r9b,$r10b,$r12b,$r13b,$r14b,$r15b,$r8bh,$r9bh,$r10bh,$r12bh,$r13bh,$r14bh,$r15bh,$r8d,$r9d,$r10d,$r12d,$r13d,$r14d,$r15d,$r8w,$r9w,$r10w,$r12w,$r13w,$r14w,$r15w,$r8wh,$r9wh,$r10wh,$r12wh,$r13wh,$r14wh,$r15wh), implicit $rsp, implicit $ssp, implicit $rdi, implicit $rsi, implicit $rdx, implicit $rcx, implicit $r8, implicit-def $rsp, implicit-def $ssp, implicit-def $rax
      ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp

The updated revision also contains a slight functional change: for functions with the `sret` parameter (function returning a struct by value) I still preserve `%rax`. The standard SysV CC requires the `sret` (i.e. `%rdi`) be copied into `%rax`, but for the `preserve_*` CCs this is redundant and is not actually used, since it's always the same as the preserved `%rdi`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143425