[SchedModel] Propagate read advance cycles to implicit operands outside instruction descriptor

[jonpa]

Summary

The SchedModel allows the addition of ReadAdvances to express that certain operands of the instructions is needed at a later point than the others. On SystemZ, this amounts to the register operand of a reg/mem instruction, given that the memory operand must first be loaded.

I discovered that in ~ 5% of the cases of expected latency adjustment, this was in effect not achieved. This problem involves the extra use operands added by regalloc for the full register, in case of a subregister usage, like:

After Coalescer:

1920B     %70.subreg_l32:addr64bit = MSR %70.subreg_l32:addr64bit, %70.subreg_l32:addr64bit
1952B     %70.subreg_l32:addr64bit = MSY %70.subreg_l32:addr64bit, %92:addr64bit, -12, $noreg :: (load 4 from %ir.scevgep18)

After RA:

2136B     renamable $r4l = MSR renamable $r4l, renamable $r4l, implicit killed $r4d, implicit-def $r4d
2144B     renamable $r4l = MSY renamable $r4l, renamable $r2d, -12, $noreg, implicit killed $r4d, implicit-def $r4d :: (load 4 from %ir.scevgep18)

Post-RA machine scheduler DAG:

SU(20):   renamable $r4l = MSR renamable $r4l, renamable $r4l, implicit $r4d, implicit-def $r4d
# preds left       : 3
# succs left       : 3
# rdefs left       : 0
Latency            : 6
Depth              : 2
Height             : 31
Predecessors:
SU(4): Out  Latency=0
SU(4): Data Latency=1 Reg=$r4l
SU(4): Data Latency=1 Reg=$r4d
Successors:
SU(21): Out  Latency=0
SU(21): Data Latency=2 Reg=$r4l
SU(21): Data Latency=6 Reg=$r4d
SU(21):   renamable $r4l = MSY renamable $r4l, renamable $r2d, -12, $noreg, implicit $r4d, implicit-def $r4d :: (load 4 from %ir.scevgep18)
# preds left       : 3
# succs left       : 3
# rdefs left       : 0
Latency            : 10
Depth              : 8
Height             : 25
Predecessors:
SU(20): Out  Latency=0
SU(20): Data Latency=2 Reg=$r4l
SU(20): Data Latency=6 Reg=$r4d

SU(20) has instruction latency 6, and MSY has a ReadAdvance on the first use operand of 4 cycles ($r4l). However, the $r4d operand is not covered by this, so the effective latency between the nodes is still 6!

It seems to me that this is a target independent problem. I am not really sure how to best handle this situation, but it seems that the patch I made here solves the problem on SystemZ.

I thought about a simpler version like "If a register use is not part of the instruction descriptor, set latency to 0, in case a subreg has a read advance". I did not dare to do this however, since I found some rare cases (not involving ReadAdvance:s), where it was actually the super reg that had a latency value, while its subreg had a latency of 0. I am guessing this is another situation involving super/sub regs not quite the same as the more common one seen above.

As before, I am not really aware of the true necessities of these extra register allocator operands, but I trust they are needed somehow (explanations welcome). Given this, I suspect there may be some simpler way of achieving this result?

[Florob]

I'm not quite sure what gave you the idea I'd be qualified to review this. My best guess is that some minor bugfixes I made to PowerPC CodeGen years ago might still show up as relatively recent. I'll leave this to minds more capable in this area.

[MatzeB]

For context: $r4d is a super register formed from $r4l+$r4h?

[MatzeB]

This is tricky. Some comments:

• Have you tried enabling subregister liveness tracking? Among other things it gets rid of the implicit-defs/uses for the full registers... (though there may be other factors influencing that decisions)
• What about just setting the latencies induced by the artifical implicit def-/uses[1] to 0?

[1] = in lack of a better way to identify them, that would be all implicit vreg defs/uses that are not part of the MCInstrDesc.

[MatzeB]

As before, I am not really aware of the true necessities of these extra register allocator operands, but I trust they are needed somehow (explanations welcome).

I think we mainly need these operands to make some situations explicit to the machine verifier and liveness computation. They are necessary to model some subreg liveness effects for the allocator when subregister liveness tracking is not enabled. Right now I wonder I cannot come up with the reason why we still keep them around with physregs after assignment (because when subreg liveness tracking is enabled we cannot even add them)...

[MatzeB]

In D49671#1172545, @MatzeB wrote:

As before, I am not really aware of the true necessities of these extra register allocator operands, but I trust they are needed somehow (explanations welcome).

I think we mainly need these operands to make some situations explicit to the machine verifier and liveness computation. They are necessary to model some subreg liveness effects for the allocator when subregister liveness tracking is not enabled. Right now I wonder I cannot come up with the reason why we still keep them around with physregs after assignment (because when subreg liveness tracking is enabled we cannot even add them)...

Ignore this comment, looking at the code we obviously do not have implicit defs/uses before regalloc and only add them in VirtRegRewriter. Right now I'm struggling to come up with the reasoning for their existence... Might be related to the block-live-in lists not being computed at subreg granularity...

[jonpa]

In D49671#1172423, @Florob wrote:

I'm not quite sure what gave you the idea I'd be qualified to review this. My best guess is that some minor bugfixes I made to PowerPC CodeGen years ago might still show up as relatively recent. I'll leave this to minds more capable in this area.

Sorry! I meant to add Florian Hahn...

[jonpa]

In D49671#1172499, @MatzeB wrote:

For context: $r4d is a super register formed from $r4l+$r4h?

correct

[jonpa]

In D49671#1172572, @MatzeB wrote:

In D49671#1172545, @MatzeB wrote:

As before, I am not really aware of the true necessities of these extra register allocator operands, but I trust they are needed somehow (explanations welcome).

I think we mainly need these operands to make some situations explicit to the machine verifier and liveness computation. They are necessary to model some subreg liveness effects for the allocator when subregister liveness tracking is not enabled. Right now I wonder I cannot come up with the reason why we still keep them around with physregs after assignment (because when subreg liveness tracking is enabled we cannot even add them)...

Ignore this comment, looking at the code we obviously do not have implicit defs/uses before regalloc and only add them in VirtRegRewriter. Right now I'm struggling to come up with the reasoning for their existence... Might be related to the block-live-in lists not being computed at subreg granularity...

Personally, I think there should be a REALLY good reason to keep them around, given that it makes things like this more involved after regalloc. It would be much nicer to just have the tablegen operands around in many cases... I thought it might have something to do with early-clobber of the other subreg or such things, although I don't have any experience with it.

[jonpa]

In D49671#1172527, @MatzeB wrote:

This is tricky. Some comments:

• Have you tried enabling subregister liveness tracking? Among other things it gets rid of the implicit-defs/uses for the full registers... (though there may be other factors influencing that decisions)

Yes, IIRC I have tried that, but got crashes immediately which was discouraging. So for the moment, that is not something that could be the default for SystemZ, I think.

• What about just setting the latencies induced by the artifical implicit def-/uses[1] to 0?

[1] = in lack of a better way to identify them, that would be all implicit vreg defs/uses that are not part of the MCInstrDesc.

Yes, that was also my idea but as I wrote earlier in some rare cases I noticed instructions where the actual latency was only put on that extra regalloc operand, while the explicit use op had just a unit latency!

I looked into this now a bit more, and it seems that in these cases a multiply or other instruction requires a double word register (128 bit), so a 64 bit register is coalesced into it:

Before Coalescing:

16B       %0:gr64bit = LGFRL @seedi ::
128B      undef %5.subreg_l64:gr128bit = COPY %0:gr64bit
144B      %6:gr128bit = COPY %5:gr128bit
160B      %6:gr128bit = MLGR %6:gr128bit, %3:gr64bit

After Coalescing:

16B       undef %5.subreg_l64:gr128bit = LGFRL @seedi :: (dereferenceable load 4 from @seedi)
...
144B      %6:gr128bit = COPY %5:gr128bit

After RA:

bb.0.entry:
renamable $r1d = LGFRL @seedi, implicit-def $r0q :: (dereferenceable load 4 from @seedi)
...
renamable $r4q = COPY renamable $r0q

After Post-RA pseudo instruction expansion pass:

bb.0.entry:
renamable $r1d = LGFRL @seedi, implicit-def $r0q :: (dereferenceable load 4 from @seedi)
...
$r4d = LGR $r0d, implicit $r0q
$r5d = LGR $r1d, implicit $r0q

DAG has the latency on $r0q (superreg), instead of $r0d between SU(0) and SU(3). ($r0q = $r0d + $r1d):

SU(0):   renamable $r1d = LGFRL @seedi, implicit-def $r0q :: (dereferenceable load 4 from @seedi)
# preds left       : 0
# succs left       : 9
# rdefs left       : 0
Latency            : 5
Depth              : 0
Height             : 70
Successors:
SU(4): Data Latency=5 Reg=$r1d
SU(4): Data Latency=5 Reg=$r0q
SU(3): Data Latency=5 Reg=$r0q
SU(3): Data Latency=1 Reg=$r0d
...
SU(3):   $r4d = LGR $r0d, implicit $r0q
# preds left       : 2
# succs left       : 3
# rdefs left       : 0
Latency            : 1
Depth              : 5
Height             : 65
Predecessors:
SU(0): Data Latency=5 Reg=$r0q
SU(0): Data Latency=1 Reg=$r0d
...
SU(4):   $r5d = LGR $r1d, implicit $r0q
# preds left       : 2
# succs left       : 3
# rdefs left       : 0
Latency            : 1
Depth              : 5
Height             : 65
Predecessors:
SU(0): Data Latency=5 Reg=$r1d
SU(0): Data Latency=5 Reg=$r0q
...

Seems like these are (rare) cases then where the defining instruction has an explicit def-op of a subregister, and a RegAlloc-implicit-def of the full register. The using instruction has an explicit use of the *other*
subreg, and an implicit use of the full register. The latency value is set only on the super-register (RegAlloc operand).

During DAG construction in computeOperandLatency(), when handling SU(0), I saw that

OperIdx 0: $r1d -> $r0q : Wlat0, Lat:5
OperIdx 2: $r0q -> $r0d : DefIdx = 1, but there is only one WriteLatencyEntry with the correct Value of 5! So the latency here becomes '1' instead. This is another example of how difficult these extra RA operands are to deal with (and it would be really ugly to have extra SchedWrites in the tablegen file just "in case regalloc decides to add one or a few more").

So, in short: we can't just set the latency on those regalloc operands to 1 whenever we want, because in these cases that would break the SchedModel. That said, this is extremely rare (35 cases out of 1.3 million) currently on SystemZ on SPEC, arising just in this scenario with a coalesced 128 bit register required by a particular instruction. So at least currently, that wouldn't probably matter if ignored... Still, maybe it would on other targets... Of course, this would currently be much better to do on SystemZ instead of the currently missing ReadAdvances...

I guess I wish that since the SchedModel has the quite intricate mapping of SchedWrites to operands (by means of ordering), it would hopefully end there, and not get disrupted with these extra operands... Defining a SchedModel to match the instruction definition operands is hard enough, and it doesn't work well to have to deal with extra implicit regalloc operands as well...

If we have to live with them on the MIs, perhaps we could make some decision to to not give them any latency values on the edges somehow, but to keep the latency values as defined by the tablegen files for the operands found there only?

That would probably be trading a "look-up" (this current patch), for another one, where an implicit operand not part of the MCInstrDesc would have to check for a subreg on the MI and get the latency from it... Not sure if that's even a good idea...

[jonpa]

Some tests were crashing. To get all tests are passing I had to:

• Avoid using MI->getMF(), because e.g. machinecombiner will call with an MI that is not contained in any MBB.

• Somehow make sure that this is only done post-RA (since this aims to handle the regalloc operands). Due to the fact that UseMI may not always be part of the MF, MRI is not retrievable, so instead of doing MRI->isSSA() around all of this, I checked for each use operands physical/virtual domain. I wonder how to improve on this...

New test case for SystemZ that tests that the latency adjustment of the read advance is also applied on the register allocator operand. The test case is a bit longer than expected after bugpoint reduction, but I think that's how it has to be to expose this effect of coalescing
into a superregister or something like that... May be able to find a smaller test case...

[jonpa]

Patch somewhat simplified with NFC on SystemZ/SPEC. SystemZ test case fixed to use '-o -', to not write the .s file to the repo.

Is this approach acceptable, or are the extra RegAlloc super-reg operands somehow under review / redesign?

[MatzeB]

In D49671#1173217, @jonpa wrote:

In D49671#1172527, @MatzeB wrote:

This is tricky. Some comments:

• Have you tried enabling subregister liveness tracking? Among other things it gets rid of the implicit-defs/uses for the full registers... (though there may be other factors influencing that decisions)

Yes, IIRC I have tried that, but got crashes immediately which was discouraging. So for the moment, that is not something that could be the default for SystemZ, I think.

• What about just setting the latencies induced by the artifical implicit def-/uses[1] to 0?

[1] = in lack of a better way to identify them, that would be all implicit vreg defs/uses that are not part of the MCInstrDesc.

Yes, that was also my idea but as I wrote earlier in some rare cases I noticed instructions where the actual latency was only put on that extra regalloc operand, while the explicit use op had just a unit latency!

I looked into this now a bit more, and it seems that in these cases a multiply or other instruction requires a double word register (128 bit), so a 64 bit register is coalesced into it:

Before Coalescing:

16B       %0:gr64bit = LGFRL @seedi ::
128B      undef %5.subreg_l64:gr128bit = COPY %0:gr64bit
144B      %6:gr128bit = COPY %5:gr128bit
160B      %6:gr128bit = MLGR %6:gr128bit, %3:gr64bit

After Coalescing:

16B       undef %5.subreg_l64:gr128bit = LGFRL @seedi :: (dereferenceable load 4 from @seedi)
...
144B      %6:gr128bit = COPY %5:gr128bit

After RA:

bb.0.entry:
renamable $r1d = LGFRL @seedi, implicit-def $r0q :: (dereferenceable load 4 from @seedi)
...
renamable $r4q = COPY renamable $r0q

After Post-RA pseudo instruction expansion pass:

bb.0.entry:
renamable $r1d = LGFRL @seedi, implicit-def $r0q :: (dereferenceable load 4 from @seedi)
...
$r4d = LGR $r0d, implicit $r0q
$r5d = LGR $r1d, implicit $r0q

DAG has the latency on $r0q (superreg), instead of $r0d between SU(0) and SU(3). ($r0q = $r0d + $r1d):

SU(0):   renamable $r1d = LGFRL @seedi, implicit-def $r0q :: (dereferenceable load 4 from @seedi)
# preds left       : 0
# succs left       : 9
# rdefs left       : 0
Latency            : 5
Depth              : 0
Height             : 70
Successors:
SU(4): Data Latency=5 Reg=$r1d
SU(4): Data Latency=5 Reg=$r0q
SU(3): Data Latency=5 Reg=$r0q
SU(3): Data Latency=1 Reg=$r0d
...
SU(3):   $r4d = LGR $r0d, implicit $r0q
# preds left       : 2
# succs left       : 3
# rdefs left       : 0
Latency            : 1
Depth              : 5
Height             : 65
Predecessors:
SU(0): Data Latency=5 Reg=$r0q
SU(0): Data Latency=1 Reg=$r0d
...
SU(4):   $r5d = LGR $r1d, implicit $r0q
# preds left       : 2
# succs left       : 3
# rdefs left       : 0
Latency            : 1
Depth              : 5
Height             : 65
Predecessors:
SU(0): Data Latency=5 Reg=$r1d
SU(0): Data Latency=5 Reg=$r0q
...

Seems like these are (rare) cases then where the defining instruction has an explicit def-op of a subregister, and a RegAlloc-implicit-def of the full register. The using instruction has an explicit use of the *other*
subreg, and an implicit use of the full register. The latency value is set only on the super-register (RegAlloc operand).

• The "implicit-def of the full register" is added when materializing the result of the regalloc in VirtRegMap.cpp, it is not present or necessary during regalloc itself.
• In your debug output I see:

SU(0):   renamable $r1d = LGFRL @seedi, implicit-def $r0q :: (dereferenceable load 4 from @seedi)
...
Successors:
SU(4): Data Latency=5 Reg=$r1d
SU(4): Data Latency=5 Reg=$r0q
SU(3): Data Latency=5 Reg=$r0q
SU(3): Data Latency=1 Reg=$r0d

Given that the actual instruction only writes to r1d I would argue that the latencies on r0q and r0d are "fake". Hence my proposal to ignore the extra operands during schedule dag construction or force their latency to zero. Or do you actually want a latency between SU(0) and SU(3) here?

During DAG construction in computeOperandLatency(), when handling SU(0), I saw that

OperIdx 0: $r1d -> $r0q : Wlat0, Lat:5
OperIdx 2: $r0q -> $r0d : DefIdx = 1, but there is only one WriteLatencyEntry with the correct Value of 5! So the latency here becomes '1' instead. This is another example of how difficult these extra RA operands are to deal with (and it would be really ugly to have extra SchedWrites in the tablegen file just "in case regalloc decides to add one or a few more").

So, in short: we can't just set the latency on those regalloc operands to 1 whenever we want, because in these cases that would break the SchedModel. That said, this is extremely rare (35 cases out of 1.3 million) currently on SystemZ on SPEC, arising just in this scenario with a coalesced 128 bit register required by a particular instruction. So at least currently, that wouldn't probably matter if ignored... Still, maybe it would on other targets... Of course, this would currently be much better to do on SystemZ instead of the currently missing ReadAdvances...

I guess I wish that since the SchedModel has the quite intricate mapping of SchedWrites to operands (by means of ordering), it would hopefully end there, and not get disrupted with these extra operands... Defining a SchedModel to match the instruction definition operands is hard enough, and it doesn't work well to have to deal with extra implicit regalloc operands as well...

If we have to live with them on the MIs, perhaps we could make some decision to to not give them any latency values on the edges somehow, but to keep the latency values as defined by the tablegen files for the operands found there only?

That would probably be trading a "look-up" (this current patch), for another one, where an implicit operand not part of the MCInstrDesc would have to check for a subreg on the MI and get the latency from it... Not sure if that's even a good idea...

[jonpa]

Thanks for review!

Given that the actual instruction only writes to r1d I would argue that the latencies on r0q and r0d are "fake". Hence my proposal to ignore the extra operands during schedule dag construction or force their latency to zero.

I tried ignoring the operands during DAG construction, but that caused a lot of machine verifier errors, since those extra operands themselves have def-use chains that of course get corrupted if ignored by the scheduler.

I updated the patch to instead follow your second suggestion - setting the latency to zero in these cases. This gives ~25 test failures across targets, which I hope will be fairly simple to update if you think this patch is acceptable. This approach seems to also fix the issue I was seeing with the read advances.

Since this is a post-RA problem, I still wonder if it would perhaps be possible to instead remove these operands at some point before the post-RA scheduler (Still don't know what they are really for)? This would be even more simple, I think.

[jonpa]

ping!

[jonpa]

ping!

[jonpa]

ping

[jonpa]

ping!

[MatzeB]

Sorry for slow response. LGTM, some nitpicks below but feel free to fix testcases and nitpicks at your own discretion.

lib/CodeGen/ScheduleDAGInstrs.cpp

240–241

How about reversing the condition and calling this:

bool ImplicitPseudoDef = (OperIdx >= DefMIDesc->getNumOperands() &&
                          !DefMIDesc->hasImplicitDefOfPhysReg(MO.getReg()));

267–269

similar here:

bool ImplicitPseudoUse = UseMIDesc && UseOp >= UseMIDesc->getNumOperands() && !UseMIDesc->hasImplicitUseOfPhysReg(*Alias);

test/CodeGen/SystemZ/misched-readadvances.mir

1

If you have the time look at: https://llvm.org/docs/MIRLangRef.html#simplifying-mir-files

This smells like you can do things like dropping the IR part, not listing the successor blocks (at least for the blocks that don't use the jumptable)...

[jonpa]

Updated patch per review. Submitting again since I had to add a (int) cast to silence compiler warning:

bool ImplicitPseudoUse =
    (UseMIDesc && UseOp >= (**(int)**UseMIDesc->getNumOperands()) &&
     !UseMIDesc->hasImplicitUseOfPhysReg(*Alias));

I would think this should be ok, right? UseOp may be -1 for ExitSU, but that doesn't matter.

Reduced the test case further.

[jonpa]

I tried to begin with the test updating, but found that at least the AMDGPU tests had a lot of repeated patterns, which I suspect isn't that much work if you know the assembly dialect, but for me it was easy to get lost.

Since this is a general improvement for any target that cares about operand read advances, I would like to ask if someone from each target please could apply the patch and do the test updating? Just mail me a patch and I'll apply it and put it up here.

I am not sure about the general agreement on test updating, but I think personally this makes collaborative sense, or?

LLVM :: CodeGen/AMDGPU/call-argument-types.ll
LLVM :: CodeGen/AMDGPU/call-preserved-registers.ll
LLVM :: CodeGen/AMDGPU/callee-special-input-sgprs.ll
LLVM :: CodeGen/AMDGPU/indirect-addressing-si.ll
LLVM :: CodeGen/AMDGPU/inline-asm.ll
LLVM :: CodeGen/AMDGPU/insert_vector_elt.ll
LLVM :: CodeGen/AMDGPU/misched-killflags.mir
LLVM :: CodeGen/AMDGPU/nested-calls.ll
LLVM :: CodeGen/AMDGPU/undefined-subreg-liverange.ll
LLVM :: CodeGen/ARM/Windows/chkstk-movw-movt-isel.ll
LLVM :: CodeGen/ARM/Windows/chkstk.ll
LLVM :: CodeGen/ARM/Windows/memset.ll
LLVM :: CodeGen/ARM/arm-and-tst-peephole.ll
LLVM :: CodeGen/ARM/arm-shrink-wrapping.ll
LLVM :: CodeGen/ARM/cortex-a57-misched-ldm-wrback.ll
LLVM :: CodeGen/ARM/cortex-a57-misched-ldm.ll
LLVM :: CodeGen/ARM/cortex-a57-misched-vldm-wrback.ll
LLVM :: CodeGen/ARM/cortex-a57-misched-vldm.ll
LLVM :: CodeGen/ARM/fp16-instructions.ll
LLVM :: CodeGen/ARM/select.ll
LLVM :: CodeGen/ARM/twoaddrinstr.ll
LLVM :: CodeGen/ARM/vcombine.ll
LLVM :: CodeGen/ARM/vuzp.ll
LLVM :: CodeGen/Hexagon/ps_call_nr.ll
LLVM :: CodeGen/Thumb2/umulo-128-legalisation-lowering.ll
LLVM :: CodeGen/Thumb2/umulo-64-legalisation-lowering.ll
LLVM :: CodeGen/X86/lsr-loop-exit-cond.ll
LLVM :: CodeGen/X86/phys-reg-local-regalloc.ll
LLVM :: CodeGen/X86/schedule-x86-64-shld.ll
LLVM :: CodeGen/X86/schedule-x86_32.ll

[jonpa]

x86 test changes lgtm

Thanks for the tests patch Simon! I also made one change to phys-reg-local-regalloc.ll that you did not update, which makes all the X86 tests green. I hope that one also looks OK to you?

[RKSimon]

test/CodeGen/X86/phys-reg-local-regalloc.ll

25 ↗

(On Diff #168616)

Sorry - missed that test - looks OK.

[rampitec]

@arsenm could you please check the tests? They seems to be mostly yours, especially with respect to calls.

[jonpa]

I have gone through the tests as best as I can since the progress was slow. I will commit this in a few days if no one objects. Please take a look and review the test changes!

One test was beyond me: Hexagon/ps_call_nr.ll, which fails with 'LLVM ERROR: invalid instruction packet: slot error'.

First difference is after the packetizer, where it seems that a call has now been bundled for some reason, which I am guessing is wrong. Not sure at all how to fix.

 *** IR Dump After Hexagon Packetize   # *** IR Dump After Hexagon Packetize
 # Machine code for function f0: NoPHI   # Machine code for function f0: NoPHI

bb.0.b0:                                bb.0.b0:
  successors: %bb.1(0x00000001); %bb.     successors: %bb.1(0x00000001); %bb.

  renamable $r0 = L2_loadri_io undef      renamable $r0 = L2_loadri_io undef
  BUNDLE implicit-def dead $p0, impli     BUNDLE implicit-def dead $p0, impli
  renamable $p0 = C2_bitsclri kille       renamable $p0 = C2_bitsclri kille
  PS_jmprettnew internal killed $p0       PS_jmprettnew internal killed $p0
  }                                       }

bb.1.b2:                                bb.1.b2:
; predecessors: %bb.0                   ; predecessors: %bb.0

  BUNDLE implicit-def $r29, implicit-     BUNDLE implicit-def $r29, implicit-
  $r29 = S2_allocframe killed $r29(       $r29 = S2_allocframe killed $r29(
  $r3 = A2_tfrsi 0                        $r3 = A2_tfrsi 0
  $r4 = A2_tfrsi 0                        $r4 = A2_tfrsi 0
  }                                       }
  BUNDLE implicit-def $r1, implicit-d     BUNDLE implicit-def $r1, implicit-d
  renamable $r1 = L2_loadri_io kill       renamable $r1 = L2_loadri_io kill
  $r0 = A2_tfrsi @g0                      $r0 = A2_tfrsi @g0
  }                                       }
  BUNDLE implicit-def $r2, implicit-d |   BUNDLE implicit-def dead $r2, impli
  renamable $r2 = S2_extractu renam       renamable $r2 = S2_extractu renam
  renamable $r1 = S2_extractu renam       renamable $r1 = S2_extractu renam
                                      >     PS_call_nr @f1, <regmask $d8 $d9
  }                                       }
  PS_call_nr @f1, <regmask $d8 $d9 $d <

# End machine code for function f0.     # End machine code for function f0.

[RKSimon]

X86 test changes (still) LGTM

[kparzysz]

Hexagon packets (bundles) have 4 slots, numbered 0..3. Each one of the three instructions (2 x S2_extractu, and PS_call_nr) can only go in slots 2 or 3, so something went horribly wrong.

Could you post the output from -debug-only=packets?

As it is now, this is very bad, so please do not commit this until we figure this out.

[jonpa]

In D49671#1277445, @kparzysz wrote:

Hexagon packets (bundles) have 4 slots, numbered 0..3. Each one of the three instructions (2 x S2_extractu, and PS_call_nr) can only go in slots 2 or 3, so something went horribly wrong.

Could you post the output from -debug-only=packets?

As it is now, this is very bad, so please do not commit this until we figure this out.

Sure, here are three files: the output with patch / without patch (base) / side-to-side diff

debug_packets.diff13 KBDownload

debug_packets.base.txt9 KBDownload

debug_packets.txt8 KBDownload

[tstellar]

In D49671#1257221, @jonpa wrote:

I tried to begin with the test updating, but found that at least the AMDGPU tests had a lot of repeated patterns, which I suspect isn't that much work if you know the assembly dialect, but for me it was easy to get lost.

Do you still need help with the AMDGPU tests?

[jonpa]

In D49671#1277479, @tstellar wrote:

In D49671#1257221, @jonpa wrote:

I tried to begin with the test updating, but found that at least the AMDGPU tests had a lot of repeated patterns, which I suspect isn't that much work if you know the assembly dialect, but for me it was easy to get lost.

Do you still need help with the AMDGPU tests?

Yes, please look through the changes I made. Thanks!

[tstellar]

AMDGPU tests look good, just the one comment for indirect-addressing-si.ll.

test/CodeGen/AMDGPU/indirect-addressing-si.ll

390 ↗

(On Diff #171255)

This [3] looks like a typo.

[jonpa]

test/CodeGen/AMDGPU/indirect-addressing-si.ll

390 ↗

(On Diff #171255)

:-)

I know that looks weird and suspected you might not like it. The problem was that the VEC_ELT1 register did not match properly further down. IIRC, there were different matches for different subtargets, so I had to force one of the matches into v3 (I suppose I should have removed the '+').

Please help me out and check if there is a better way, or if this is acceptable.

[tstellar]

test/CodeGen/AMDGPU/indirect-addressing-si.ll

390 ↗

(On Diff #171255)

Ok, that's fine. I would remove the + and also the brackets.

[bcahoon]

In D49671#1277445, @kparzysz wrote:

Hexagon packets (bundles) have 4 slots, numbered 0..3. Each one of the three instructions (2 x S2_extractu, and PS_call_nr) can only go in slots 2 or 3, so something went horribly wrong.

As it is now, this is very bad, so please do not commit this until we figure this out.

I committed a patch that fixes this problem, so ps_call_nr.ll should pass now.

[jonpa]

In D49671#1278005, @bcahoon wrote:

In D49671#1277445, @kparzysz wrote:

Hexagon packets (bundles) have 4 slots, numbered 0..3. Each one of the three instructions (2 x S2_extractu, and PS_call_nr) can only go in slots 2 or 3, so something went horribly wrong.

As it is now, this is very bad, so please do not commit this until we figure this out.

I committed a patch that fixes this problem, so ps_call_nr.ll should pass now.

Thanks, I have confirmed that the test case passes now with the patch.

Now that the X86, AMDGPU and Hexagon test failures are handled, only the ARM and Thumb2 test updates are left to be reviewed, please.

[jonpa]

Thanks for review. r345606.

Note: Two minor last-minute regenerations of X86 tests: CodeGen/X86/memset.ll and CodeGen/X86/schedule-x86-64-shld.ll

[materi]

This patch is causing some problems in my out-of-tree back-end. We add some MachineOperands on the fly for some uses/defs that are conditional or depend on some circumstances, like how registers were allocated, or which depth a loop is at in a loop nest. With this patch, these manually added operands don't work as we intend.

I'm wondering if we could maybe keep the old flexible way to look at MachineOperands and put the functionality which sets the latency to zero in the getOperandLatency hook instead?

I also think it's a bad idea to have some MachineOperands be special but it's not visible in the debug printouts. Now we have to read the tablegen file to understand if an operand is considered fake or not.

[SjoerdMeijer]

Apologies for being late to the party, but I am now looking into this too because we've seen some significant regressions with this change committed.
I am not blaming this commit, not yet, because I haven't fully understood the problem yet. As I am new to this area, I wanted to dump some initial thoughts here (because it takes me some time to get up to speed), perhaps people can comment.

First, we found the change in test CodeGen/ARM/cortex-a57-misched-ldm-wrback.ll a bit suspicious. Latencies are changed from 1, 3, 3, and 4 to:

; CHECK-SAME:  Latency=1
; CHECK-NEXT:  Data
; CHECK-SAME:  Latency=3
; CHECK-NEXT:  Data
; CHECK-SAME:  Latency=0
; CHECK-NEXT:  Data
; CHECK-SAME:  Latency=0

The last 2 latencies are changed to 0. We are generating a LDM for this case: ldm r0!, {r1, r2, r3}, and I don't see yet why the latency of the last 2 operands are 0s.

This makes us wonder if variadic instructions and instructions with optional defs are ignored/missed in this patch?

[jonpa]

This patch is causing some problems in my out-of-tree back-end
...
I'm wondering if we could maybe keep the old flexible way to look at MachineOperands and put the functionality which sets the latency to zero in the getOperandLatency hook instead?

This makes us wonder if variadic instructions and instructions with optional defs are ignored/missed in this patch?

Sorry to hear about the problems!

@materi: I think your ideas make sense. If you have a patch, could you post it, please?

I think me and @MatzeB (correct me if I am wrong) may have overlooked this. We were discussing if those extra regalloc operands were needed anywhere, and how much easier life would be without them in cases like this. This was because it doesn't make sense to handle non-tablegen operands in the Schedmodel description. So we removed the latencies for the superregs since they were redundant, and forgot about *other* pseudo implicit operands that are *not* redundant.

My first idea for this patch was to loop over the operands / read advances of the instruction in order to propagate read advances to the non-tablegen super-reg operands (see earlier patch proposal under 'History'). This is more arduous than the simply clearing those latencies during DAG construction, per what was committed. I think however this should work for you as well, or?

I wonder if it would be enough to make a rule to clear latencies only on *implicit* extra operands, and not on explicit ones? In other words if added *explicit* operands were left alone, this would not break anything? But I am not sure if this is possible with variadic instructions, or if it's a good idea...

[materi]

In D49671#1283621, @jonpa wrote:

I'm wondering if we could maybe keep the old flexible way to look at MachineOperands and put the functionality which sets the latency to zero in the getOperandLatency hook instead?

@materi: I think your ideas make sense. If you have a patch, could you post it, please?

What I meant was to have subtargets override the default latency calculation if they want to ignore operands that are not in the MCInstrDesc. That is, implement SytemZ::getOperandLatency() and have it return 0 in the cases where DefIdx or UseIdx is too large.

I don't have a patch for this and I really don't know which targets want the new behavior.

My first idea for this patch was to loop over the operands / read advances of the instruction in order to propagate read advances to the non-tablegen super-reg operands (see earlier patch proposal under 'History'). This is more arduous than the simply clearing those latencies during DAG construction, per what was committed. I think however this should work for you as well, or?

I wonder if it would be enough to make a rule to clear latencies only on *implicit* extra operands, and not on explicit ones? In other words if added *explicit* operands were left alone, this would not break anything? But I am not sure if this is possible with variadic instructions, or if it's a good idea...

I don't like any implementation that has first-class and second-class MachineOperands. At least I think it's a bad idea to have this in the default implementation, doing it in a target hook makes sense though.

[jonpa]

I don't have a patch for this and I really don't know which targets wants the new behavior.

I think this behavior is wanted by all targets that define a SchedModel that includes ReadAdvances as well as using subregisters. This seems to include AArch64 , ARM, X86 and SystemZ, as far as I can tell.

Therefore I think this would be worth resolving in common code...

[MatzeB]

In D49671#1283704, @materi wrote:

In D49671#1283621, @jonpa wrote:

I'm wondering if we could maybe keep the old flexible way to look at MachineOperands and put the functionality which sets the latency to zero in the getOperandLatency hook instead?

@materi: I think your ideas make sense. If you have a patch, could you post it, please?

What I meant was to have subtargets override the default latency calculation if they want to ignore operands that are not in the MCInstrDesc. That is, implement SytemZ::getOperandLatency() and have it return 0 in the cases where DefIdx or UseIdx is too large.

I don't have a patch for this and I really don't know which targets want the new behavior.

My first idea for this patch was to loop over the operands / read advances of the instruction in order to propagate read advances to the non-tablegen super-reg operands (see earlier patch proposal under 'History'). This is more arduous than the simply clearing those latencies during DAG construction, per what was committed. I think however this should work for you as well, or?

I wonder if it would be enough to make a rule to clear latencies only on *implicit* extra operands, and not on explicit ones? In other words if added *explicit* operands were left alone, this would not break anything? But I am not sure if this is possible with variadic instructions, or if it's a good idea...

I don't like any implementation that has first-class and second-class MachineOperands. At least I think it's a bad idea to have this in the default implementation, doing it in a target hook makes sense though.

I don't think having a target specific hook is good enough here because some of the problematic operands are generated by generic register allocation code; you will get them as soon as you have subregisters in the mix.

Some words about the different kinds of operands:

The extra operands do make sense semantically and are necessary for our modeling of things. The thing I regret though is that just being an implicit operand can mean two things today: It's an operand that isn't explicitly emitted to assembly/encoded or it's an operand that does not correspond to a read/write access in hardware or both. In this patch we only want to catch the 2nd kind, but not purely cases of the first. While it is unfortunate to not have this modeled as two separate bits today, it feels to me like the heuristic is close enough. Should we try again with an extra MO.isImplicit() in the condition?

[MatzeB]

In D49671#1282086, @materi wrote:

This patch is causing some problems in my out-of-tree back-end. We add some MachineOperands on the fly for some uses/defs that are conditional or depend on some circumstances, like how registers were allocated, or which depth a loop is at in a loop nest. With this patch, these manually added operands don't work as we intend.

Would you be in a position to mark your extra operands as explicit operands since the machine does appear to be reading/writing them in your case? (Maybe you have to mark the instruction as variadic, or I would be open to invent a new MCInstrDesc flag if that helps...)

I'm wondering if we could maybe keep the old flexible way to look at MachineOperands and put the functionality which sets the latency to zero in the getOperandLatency hook instead?

I also think it's a bad idea to have some MachineOperands be special but it's not visible in the debug printouts. Now we have to read the tablegen file to understand if an operand is considered fake or not.

I don't like it either, but this fact is true independently of this patch...

[materi]

In D49671#1284997, @MatzeB wrote:

In D49671#1283704, @materi wrote:

I don't like any implementation that has first-class and second-class MachineOperands. At least I think it's a bad idea to have this in the default implementation, doing it in a target hook makes sense though.

I don't think having a target specific hook is good enough here because some of the problematic operands are generated by generic register allocation code; you will get them as soon as you have subregisters in the mix.

Hmm, also if you use TracksSubRegLiveness?

Some words about the different kinds of operands:

The extra operands do make sense semantically and are necessary for our modeling of things. The thing I regret though is that just being an implicit operand can mean two things today: It's an operand that isn't explicitly emitted to assembly/encoded or it's an operand that does not correspond to a read/write access in hardware or both. In this patch we only want to catch the 2nd kind, but not purely cases of the first. While it is unfortunate to not have this modeled as two separate bits today, it feels to me like the heuristic is close enough. Should we try again with an extra MO.isImplicit() in the condition?

I agree that this overloading is unfortunate! How hard would it be to split these operands in two different types?

I also wonder if the only purpose of implicit operands added in regalloc is for liveness modeling? What about things like AH/AL on x86 where writing a subregister spills over to the neighbor?

In D49671#1284998, @MatzeB wrote:

In D49671#1282086, @materi wrote:

This patch is causing some problems in my out-of-tree back-end. We add some MachineOperands on the fly for some uses/defs that are conditional or depend on some circumstances, like how registers were allocated, or which depth a loop is at in a loop nest. With this patch, these manually added operands don't work as we intend.

Would you be in a position to mark your extra operands as explicit operands since the machine does appear to be reading/writing them in your case? (Maybe you have to mark the instruction as variadic, or I would be open to invent a new MCInstrDesc flag if that helps...)

Yes I think we could change them to explicit operands. I have not tried, but I can't think of any reason it would not work.

I'm wondering if we could maybe keep the old flexible way to look at MachineOperands and put the functionality which sets the latency to zero in the getOperandLatency hook instead?

I also think it's a bad idea to have some MachineOperands be special but it's not visible in the debug printouts. Now we have to read the tablegen file to understand if an operand is considered fake or not.

I don't like it either, but this fact is true independently of this patch...

As I understand it, once the operand is attached to the MI, LLVM treated it the same regardless where it came from. So the semantics of the operand was obvious from looking at MI->dump().

[bjope]

Maybe the register allocator should add the implicit-def as an IMPLICIT_DEF instruction just before the MI instead of attaching a bogus impl def on the MI (if the MI uses parts of the register I guess the IMPLICIT_DEF needs a corresponding implicit use).

So instead of

After Coalescing:

16B       undef %5.subreg_l64:gr128bit = LGFRL @seedi :: (dereferenceable load 4 from @seedi)

After RA:

renamable $r1d = LGFRL @seedi, implicit-def $r0q :: (dereferenceable load 4 from @seedi)

you would get

After Coalescing:

16B       undef %5.subreg_l64:gr128bit = LGFRL @seedi :: (dereferenceable load 4 from @seedi)

After RA:

$r0q = IMPLICIT_DEF
renamable $r1d = LGFRL @seedi :: (dereferenceable load 4 from @seedi)

This way the definitions added to LGFRL maps to the actual definition done by the instruction. The "fake" implicit defs is separated from the LGFRL instruction and the scheduler wouldn't see the false dependency toward LGFRL (instead it would have DAG edge towards the IMPLICIT_DEF but I guess that would get zero latency?). Or wouldn't that help both the problem that this ticket was aiming at solving, and the problems that @materi is describing?

So there would be a slight difference between implicit-def operands and IMPLICIT_DEF:

• An implicit-def operand would be seen as a side effect performed by the instruction.
• An IMPLICIT_DEF instruction would be used to model that a register is undefined (for liveness purposes).

For the record, I haven't put too much thought into the above. But afaict we can have IMPLICIT_DEF instructions after RA already today. Maybe the problem is that RA passes are adding implicit defs to real instructions, when they actually should add IMPLICIT_DEF instructions instead when modelling undef?

[materi]

In D49671#1285281, @materi wrote:

In D49671#1284997, @MatzeB wrote:

Some words about the different kinds of operands:

The extra operands do make sense semantically and are necessary for our modeling of things. The thing I regret though is that just being an implicit operand can mean two things today: It's an operand that isn't explicitly emitted to assembly/encoded or it's an operand that does not correspond to a read/write access in hardware or both. In this patch we only want to catch the 2nd kind, but not purely cases of the first. While it is unfortunate to not have this modeled as two separate bits today, it feels to me like the heuristic is close enough. Should we try again with an extra MO.isImplicit() in the condition?

I agree that this overloading is unfortunate! How hard would it be to split these operands in two different types?

I tried adding a bit in MachineOperand which is set by VirtRegMap when handling SuperDefs/SuperKills/SuperDeads. That makes it possible to filter out that kind of dependency in addPhysRegDeps. It seems to work fine and avoids having to look at MCInstrDesc.

Adding a bit in MachineOperand increases the size of the class; I don't know if that's a big deal.

In D49671#1294302, @bjope wrote:

For the record, I haven't put too much thought into the above. But afaict we can have IMPLICIT_DEF instructions after RA already today. Maybe the problem is that RA passes are adding implicit defs to real instructions, when they actually should add IMPLICIT_DEF instructions instead when modelling undef?

I think this is an interesting idea. But does this work without enabling TracksSubRegLiveness if LivenessAfterRegalloc is enabled? (This is so complicated! Maybe it's not an issue at all?)

Anyway, I think that the current code is broken and may miscompile code with SDNPVariadic instructions.

lib/CodeGen/ScheduleDAGInstrs.cpp

 /// data dependencies from SU to any uses of the physical register.
 void ScheduleDAGInstrs::addPhysRegDataDeps(SUnit *SU, unsigned OperIdx) {
   const MachineOperand &MO = SU->getInstr()->getOperand(OperIdx);
   assert(MO.isDef() && "expect physreg def");
 
   // Ask the target if address-backscheduling is desirable, and if so how much.
   const TargetSubtargetInfo &ST = MF.getSubtarget();
 
+  // Only use any non-zero latency for real defs/uses, in contrast to
+  // "fake" operands added by regalloc.
+  const MCInstrDesc *DefMIDesc = &SU->getInstr()->getDesc();
+  bool DescriptorDef = (OperIdx < DefMIDesc->getNumOperands() ||
+                        DefMIDesc->hasImplicitDefOfPhysReg(MO.getReg()));
   for (MCRegAliasIterator Alias(MO.getReg(), TRI, true);
        Alias.isValid(); ++Alias) {
     if (!Uses.contains(*Alias))
       continue;
     for (Reg2SUnitsMap::iterator I = Uses.find(*Alias); I != Uses.end(); ++I) {
       SUnit *UseSU = I->SU;
       if (UseSU == SU)
         continue;
 
       // Adjust the dependence latency using operand def/use information,
       // then allow the target to perform its own adjustments.
       int UseOp = I->OpIdx;
       MachineInstr *RegUse = nullptr;
       SDep Dep;
       if (UseOp < 0)
         Dep = SDep(SU, SDep::Artificial);
       else {
         // Set the hasPhysRegDefs only for physreg defs that have a use within
         // the scheduling region.
         SU->hasPhysRegDefs = true;
         Dep = SDep(SU, SDep::Data, *Alias);
         RegUse = UseSU->getInstr();
       }
-      Dep.setLatency(
-        SchedModel.computeOperandLatency(SU->getInstr(), OperIdx, RegUse,
-                                         UseOp));
-
+      const MCInstrDesc *UseMIDesc =
+          (RegUse ? &UseSU->getInstr()->getDesc() : nullptr);
+      bool DescriptorUse =
+          (!UseMIDesc || (UseOp < UseMIDesc->getNumOperands() ||
+                          UseMIDesc->hasImplicitUseOfPhysReg(*Alias)));
+      if (DescriptorDef && DescriptorUse) {
+        Dep.setLatency(SchedModel.computeOperandLatency(SU->getInstr(), OperIdx,
+                                                        RegUse, UseOp));
         ST.adjustSchedDependency(SU, UseSU, Dep);
+      } else
+        Dep.setLatency(0);
+
       UseSU->addPred(Dep);
     }
   }
 }
 
 /// Adds register dependencies (data, anti, and output) from this SUnit
 /// to following instructions in the same scheduling region that depend the
 /// physical register referenced at OperIdx.

test/CodeGen/SystemZ/misched-readadvances.mir

+# Check that the latency adjustment (ReadAdvance) for the MSY register operand
+# is also used on the extra operand for the full register added by RegAlloc.
+
+# RUN: llc %s -mtriple=s390x-linux-gnu -mcpu=z13 -start-before=machine-scheduler \
+# RUN:  -debug-only=machine-scheduler -o - 2>&1 | FileCheck %s
+# REQUIRES: asserts
+
+# CHECK: ScheduleDAGMI::schedule starting
+# CHECK: SU(3): renamable $r2l = MSR renamable $r2l, renamable $r2l
+# CHECK:   Latency : 6
+# CHECK: SU(4): renamable $r2l = MSY renamable $r2l, renamable $r1d, -4, $noreg, implicit $r2d
+# CHECK:   Predecessors:
+# CHECK:     SU(3): Data Latency=2 Reg=$r2l
+# CHECK:     SU(3): Data Latency=0 Reg=$r2d
+
+--- |
+  ; Function Attrs: nounwind
+  define dso_local void @Perl_do_sv_dump() local_unnamed_addr #0 {
+  bb:
+    %0 = zext i8 undef to i32
+    switch i32 %0, label %bb4 [
+      i32 12, label %bb1
+      i32 14, label %bb1
+      i32 11, label %bb2
+      i32 13, label %bb3
+      i32 8, label %bb5
+    ]
+  
+  bb1:                                              ; preds = %bb, %bb
+    unreachable
+  
+  bb2:                                              ; preds = %bb
+    %1 = zext i8 undef to i32
+    switch i32 %1, label %bb22 [
+      i32 0, label %bb6
+      i32 1, label %bb7
+      i32 2, label %bb8
+      i32 3, label %bb9
+      i32 4, label %bb10
+      i32 5, label %bb11
+      i32 6, label %bb12
+      i32 8, label %bb13
+      i32 7, label %bb14
+      i32 9, label %bb15
+      i32 10, label %bb16
+      i32 11, label %bb17
+      i32 12, label %bb18
+      i32 13, label %bb19
+      i32 14, label %bb20
+      i32 15, label %bb21
+    ]
+  
+  bb3:                                              ; preds = %bb
+    unreachable
+  
+  bb4:                                              ; preds = %bb
+    unreachable
+  
+  bb5:                                              ; preds = %bb
+    unreachable
+  
+  bb6:                                              ; preds = %bb2
+    ret void
+  
+  bb7:                                              ; preds = %bb2
+    unreachable
+  
+  bb8:                                              ; preds = %bb2
+    %2 = zext i8 undef to i32
+    switch i32 %2, label %bb46 [
+      i32 9, label %bb23
+      i32 10, label %bb24
+      i32 11, label %bb25
+      i32 15, label %bb45
+      i32 14, label %bb43
+      i32 13, label %bb44
+    ]
+  
+  bb9:                                              ; preds = %bb2
+    unreachable
+  
+  bb10:                                             ; preds = %bb2
+    unreachable
+  
+  bb11:                                             ; preds = %bb2
+    unreachable
+  
+  bb12:                                             ; preds = %bb2
+    unreachable
+  
+  bb13:                                             ; preds = %bb2
+    unreachable
+  
+  bb14:                                             ; preds = %bb2
+    unreachable
+  
+  bb15:                                             ; preds = %bb2
+    unreachable
+  
+  bb16:                                             ; preds = %bb2
+    unreachable
+  
+  bb17:                                             ; preds = %bb2
+    unreachable
+  
+  bb18:                                             ; preds = %bb2
+    unreachable
+  
+  bb19:                                             ; preds = %bb2
+    unreachable
+  
+  bb20:                                             ; preds = %bb2
+    unreachable
+  
+  bb21:                                             ; preds = %bb2
+    unreachable
+  
+  bb22:                                             ; preds = %bb2
+    unreachable
+  
+  bb23:                                             ; preds = %bb8
+    unreachable
+  
+  bb24:                                             ; preds = %bb8
+    unreachable
+  
+  bb25:                                             ; preds = %bb8
+    br label %bb27
+  
+  bb26:                                             ; preds = %bb27
+    %tmp = fdiv double undef, %tmp40
+    unreachable
+  
+  bb27:                                             ; preds = %bb27, %bb25
+    %lsr.iv4 = phi [200 x i32]* [ %4, %bb27 ], [ undef, %bb25 ]
+    %lsr.iv1 = phi i64 [ %lsr.iv.next2, %bb27 ], [ undef, %bb25 ]
+    %tmp29 = phi double [ %tmp40, %bb27 ], [ 0.000000e+00, %bb25 ]
+    %lsr.iv45 = bitcast [200 x i32]* %lsr.iv4 to i32*
+    %tmp30 = fadd double %tmp29, 0.000000e+00
+    %scevgep6 = getelementptr i32, i32* %lsr.iv45, i64 -1
+    %tmp33 = load i32, i32* %scevgep6, align 4
+    %3 = add i64 %lsr.iv1, 4294967291
+    %tmp3 = trunc i64 %3 to i32
+    %tmp35 = mul i32 %tmp3, %tmp3
+    %tmp36 = mul i32 %tmp35, %tmp33
+    %tmp37 = sitofp i32 %tmp36 to double
+    %tmp38 = fadd double %tmp30, %tmp37
+    %tmp39 = fadd double %tmp38, 0.000000e+00
+    %tmp40 = fadd double %tmp39, 0.000000e+00
+    %lsr.iv.next2 = add i64 %lsr.iv1, -4
+    %scevgep = getelementptr [200 x i32], [200 x i32]* %lsr.iv4, i64 0, i64 -4
+    %4 = bitcast i32* %scevgep to [200 x i32]*
+    %tmp42 = icmp sgt i64 %lsr.iv.next2, 4
+    br i1 %tmp42, label %bb27, label %bb26
+  
+  bb43:                                             ; preds = %bb8
+    unreachable
+  
+  bb44:                                             ; preds = %bb8
+    unreachable
+  
+  bb45:                                             ; preds = %bb8
+    unreachable
+  
+  bb46:                                             ; preds = %bb8
+    unreachable
+  }
+  
+  attributes #0 = { nounwind "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="z13" "target-features"="+transactional-execution,+vector,+vector-enhancements-1" "unsafe-fp-math"="false" "use-soft-float"="false" }
+  
+  !llvm.ident = !{!0}
+  
+  !0 = !{!"clang version 7.0.0 (http://llvm.org/git/clang.git 5a987ef0b0d883c8f33502e47b9b6f32e0546980) (llvm/llvm-dev-2 830762b5123b0e46a4433ceb262fce551228c9be)"}
+
+...
+---
+name:            Perl_do_sv_dump
+alignment:       4
+tracksRegLiveness: true
+registers:       
+  - { id: 0, class: addr64bit }
+  - { id: 1, class: addr64bit }
+  - { id: 2, class: vr64bit }
+  - { id: 3, class: vr64bit }
+  - { id: 4, class: gr64bit }
+  - { id: 5, class: gr64bit }
+  - { id: 6, class: grx32bit }
+  - { id: 7, class: grx32bit }
+  - { id: 8, class: grx32bit }
+  - { id: 9, class: gr64bit }
+  - { id: 10, class: grx32bit }
+  - { id: 11, class: addr64bit }
+  - { id: 12, class: addr64bit }
+  - { id: 13, class: addr64bit }
+  - { id: 14, class: grx32bit }
+  - { id: 15, class: gr64bit }
+  - { id: 16, class: grx32bit }
+  - { id: 17, class: addr64bit }
+  - { id: 18, class: addr64bit }
+  - { id: 19, class: addr64bit }
+  - { id: 20, class: gr64bit }
+  - { id: 21, class: gr64bit }
+  - { id: 22, class: vr64bit }
+  - { id: 23, class: fp64bit }
+  - { id: 24, class: gr64bit }
+  - { id: 25, class: gr64bit }
+  - { id: 26, class: fp64bit }
+  - { id: 27, class: vr64bit }
+  - { id: 28, class: addr64bit }
+  - { id: 29, class: gr32bit }
+  - { id: 30, class: gr32bit }
+  - { id: 31, class: gr32bit }
+  - { id: 32, class: fp64bit }
+  - { id: 33, class: vr64bit }
+  - { id: 34, class: vr64bit }
+  - { id: 35, class: addr64bit }
+  - { id: 36, class: addr64bit }
+  - { id: 37, class: vr64bit }
+jumpTable:       
+  kind:            block-address
+  entries:         
+    - id:              0
+      blocks:          [ '%bb.9', '%bb.10', '%bb.13', '%bb.14', '%bb.15', 
+                         '%bb.16', '%bb.18', '%bb.17', '%bb.19', '%bb.20', 
+                         '%bb.21', '%bb.22', '%bb.23', '%bb.24', '%bb.25' ]
+    - id:              1
+      blocks:          [ '%bb.27', '%bb.26', '%bb.26', '%bb.32', '%bb.31', 
+                         '%bb.33' ]
+body:             |
+  bb.0.bb:
+    successors: %bb.4(0x7ffffffb), %bb.1(0x00000005)
+  
+    %6:grx32bit = LHIMux 0
+    CHIMux %6, 0, implicit-def $cc
+    BRC 14, 6, %bb.4, implicit $cc
+    J %bb.1
+  
+  bb.1.bb:
+    successors: %bb.7(0x3ffffffe), %bb.2(0x40000002)
+    liveins: $cc
+  
+    BRC 14, 6, %bb.7, implicit killed $cc
+    J %bb.2
+  
+  bb.2.bb:
+    successors: %bb.6(0x7ffffff6), %bb.3(0x0000000a)
+  
+    %8:grx32bit = LHIMux 0
+    CHIMux %8, 0, implicit-def $cc
+    BRC 14, 6, %bb.6, implicit killed $cc
+    J %bb.3
+  
+  bb.3.bb1:
+    successors: 
+  
+  
+  bb.4.bb2:
+    successors: %bb.8(0x3ffffff9), %bb.5(0x40000007)
+  
+    %10:grx32bit = LHIMux 1
+    CHIMux %10, 0, implicit-def $cc
+    BRC 14, 6, %bb.8, implicit killed $cc
+  
+  bb.5.bb2:
+    successors: %bb.9(0x00000002), %bb.10(0x7fffffe4), %bb.13(0x00000002), %bb.14(0x00000002), %bb.15(0x00000002), %bb.16(0x00000002), %bb.18(0x00000002), %bb.17(0x00000002), %bb.19(0x00000002), %bb.20(0x00000002), %bb.21(0x00000002), %bb.22(0x00000002), %bb.23(0x00000002), %bb.24(0x00000002), %bb.25(0x00000002)
+  
+    %9:gr64bit = LLILF 4294967295
+    %11:addr64bit = SLLG %9, $noreg, 3
+    %12:addr64bit = LARL %jump-table.0
+    %13:addr64bit = LG %11, 0, %12 :: (load 8 from jump-table)
+    BR %13
+  
+  bb.6.bb3:
+    successors: 
+  
+  
+  bb.7.bb5:
+    successors: 
+  
+  
+  bb.8.bb6:
+    Return
+  
+  bb.9.bb7:
+    successors: 
+  
+  
+  bb.10.bb8:
+    successors: %bb.28(0x7ffffffa), %bb.11(0x00000006)
+  
+    %14:grx32bit = LHIMux 0
+    CHIMux %14, 0, implicit-def $cc
+    BRC 14, 6, %bb.28, implicit killed $cc
+    J %bb.11
+  
+  bb.11.bb8:
+    successors: %bb.26(0x00000002), %bb.12(0x7ffffffe)
+  
+    %16:grx32bit = LHIMux 1
+    CHIMux %16, 0, implicit-def $cc
+    BRC 14, 6, %bb.26, implicit killed $cc
+  
+  bb.12.bb8:
+    successors: %bb.27(0x20000000), %bb.26(0x00000000), %bb.32(0x20000000), %bb.31(0x20000000), %bb.33(0x20000000)
+  
+    %15:gr64bit = LLILF 4294967286
+    %17:addr64bit = SLLG %15, $noreg, 3
+    %18:addr64bit = LARL %jump-table.1
+    %19:addr64bit = LG %17, 0, %18 :: (load 8 from jump-table)
+    BR %19
+  
+  bb.13.bb9:
+    successors: 
+  
+  
+  bb.14.bb10:
+    successors: 
+  
+  
+  bb.15.bb11:
+    successors: 
+  
+  
+  bb.16.bb12:
+    successors: 
+  
+  
+  bb.17.bb13:
+    successors: 
+  
+  
+  bb.18.bb14:
+    successors: 
+  
+  
+  bb.19.bb15:
+    successors: 
+  
+  
+  bb.20.bb16:
+    successors: 
+  
+  
+  bb.21.bb17:
+    successors: 
+  
+  
+  bb.22.bb18:
+    successors: 
+  
+  
+  bb.23.bb19:
+    successors: 
+  
+  
+  bb.24.bb20:
+    successors: 
+  
+  
+  bb.25.bb21:
+    successors: 
+  
+  
+  bb.26.bb23:
+    successors: 
+  
+  
+  bb.27.bb24:
+    successors: 
+  
+  
+  bb.28.bb25:
+    %23:fp64bit = LZDR
+    %35:addr64bit = IMPLICIT_DEF
+    %36:addr64bit = IMPLICIT_DEF
+    %37:vr64bit = COPY %23
+    J %bb.30
+  
+  bb.29.bb26:
+    successors: 
+  
+  
+  bb.30.bb27:
+    successors: %bb.30(0x7fffffff), %bb.29(0x00000001)
+  
+    %27:vr64bit = WFADB %37, %23
+    %4:gr64bit = LAY %36, -4, $noreg
+    %36:addr64bit = ALGFI %36, 4294967291, implicit-def dead $cc
+    %36.subreg_l32:addr64bit = MSR %36.subreg_l32, %36.subreg_l32
+    %36.subreg_l32:addr64bit = MSY %36.subreg_l32, %35, -4, $noreg :: (load 4 from %ir.scevgep6)
+    %32:fp64bit = CDFBR %36.subreg_l32
+    %33:vr64bit = WFADB %27, %32
+    %34:vr64bit = WFADB %33, %23
+    %37:vr64bit = WFADB %34, %23
+    %35:addr64bit = LAY %35, -16, $noreg
+    CGHI %4, 4, implicit-def $cc
+    %36:addr64bit = COPY %4
+    BRC 14, 2, %bb.30, implicit killed $cc
+    J %bb.29
+  
+  bb.31.bb43:
+    successors: 
+  
+  
+  bb.32.bb44:
+    successors: 
+  
+  
+  bb.33.bb45:
+
+...