[SystemZ] Register clobber in L128 expansion

[nikic]

Running https://gist.github.com/nikic/974087a15f48f4495f25361bae3015d2 through llc -mtriple=s390x-- gives something like this:

  lg  %r4, 14920(%r4,%r15)            # 16-byte Folded Reload
  lg  %r5, 14928(%r4,%r15)            # 16-byte Folded Reload

Note that %r4 is used in the load offset calculation, but also one of the result registers.

Originally, this is a L128:

  renamable $r2q = L128 $r15d, 14920, killed $r2d :: (load (s128) from %stack.13, align 8)

That gets expanded into two LG, resulting in the register clobber:

  $r2d = LG $r15d, 14920, $r2d :: (load (s128) from %stack.13, align 8)
  $r3d = LG $r15d, 14928, killed $r2d :: (load (s128) from %stack.13, align 8)

[nikic]

It seems like L128 doesn't get produced for "normal" use of i128, but only for spills from the register allocator(?), so it's probably easier to consider this directly in MIR. The following test case is for -run-pass=postrapseudos:

--- |
  target triple = "s390x-unknown-unknown"

  define void @test() {
    ret void
  }

...
---
name:            'test'
body:             |
  bb.0:
    liveins: $r4d, $r15d
    $r4q = L128 $r15d, 14920, killed $r4d
    Return

...

Results ins:

body:             |
  bb.0:
    liveins: $r4d, $r15d
  
    $r4d = LG $r15d, 14920, $r4d
    $r5d = LG $r15d, 14928, killed $r4d
    Return

[nikic]

cc @JonPsson1 @uweigand

A possible fix would be to swap the two instructions in this case. Though one could still have a situation like

    $r4q = L128 $r5d, 14920, killed $r4d

where neither order would be correct. But I assume that can't happen in practice, at least as L128 is used currently.

[JonPsson1]

cc @JonPsson1 @uweigand

A possible fix would be to swap the two instructions in this case. Though one could still have a situation like

    $r4q = L128 $r5d, 14920, killed $r4d

where neither order would be correct. But I assume that can't happen in practice, at least as L128 is used currently.

Looks to me like you meant to write L128 with $r15d (SP) and not $r5d..?

I agree that it seems correct to change the order of the loads, and also add an assert either that the base register is the SP, or that it is not a subreg of the destination quadword reg.

[uweigand]

Looks to me like you meant to write L128 with $r15d (SP) and not $r5d..?

I think @nikic did mean $r5d - if base and index happen to be the two registers that make up the GR128 (in this case $r4q made up of $r4d and $r5d), there is no way to split the load into component loads without a temporary register.

However, I believe there's no need to support this case, as this instruction is only ever used to access spill slots, so at least of of base and index (typically base, but we probably don't have to assume that) will be either the stack or the frame pointer, which cannot be part of any GR128 that is in use.

I think we should just check whether one of base or index overlap the output and adjust the sequence to accommodate. If both base or index overlap, this should just fail an assertion.

[JonPsson1]

I think @nikic did mean $r5d - if base and index happen to be the two registers that make up the GR128 (in this case $r4q made up of $r4d and $r5d), there is no way to split the load into component loads without a temporary register.

Ah, I see the point now. Patch proposed (see above).

[nikic]

/cherry-pick d6ee7e8

[llvmbot]

/pull-request #92221