<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">On 01/28/2016 01:03 PM, Marcello
Maggioni via llvm-commits wrote:<br>
</div>
<blockquote
cite="mid:differential-rev-PHID-DREV-gklyrojh6yjaoqetv2r7-req@reviews.llvm.org"
type="cite">
<pre wrap="">kariddi created this revision.
kariddi added a reviewer: MatzeB.
kariddi added a subscriber: llvm-commits.
kariddi set the repository for this revision to rL LLVM.
Herald added subscribers: qcolombet, MatzeB.
Hello,
In our target we sometimes load multiple constants with the same instruction into registers that have disjoint subregisters and then use the subregisters separately.
In certain cases during constant re-materialization in the register coalescer the one of these multiple constants load instructions can be selected to rematerialize only one of the constants.
The situation is something like this:
vreg1 = LOAD CONSTANTS 5, 8 ; Loading both 5 and 8 in different subregs
; Copying only part of the register here, but the rest is undef.
vreg2:sub_16bit<def, read-undef> = COPY vreg1:sub_16bit
==>
; Materialize all the constants but only using one eventually.
vreg2 = LOAD_CONSTANTS 5, 8
In the above case we had an undefined registers that gets partially defined by the copy of one of the two constants in VREG1.
The re-materialization substitutes the copy with the load instructions defining the whole register now.
This is fine, because the register was undefined, but if this happens we need to update the sub-register liveness to keep track that there is a dead definition of the lanes that weren't loaded previously.
Repository:
rL LLVM
<a class="moz-txt-link-freetext" href="http://reviews.llvm.org/D16693">http://reviews.llvm.org/D16693</a>
Files:
lib/CodeGen/MachineVerifier.cpp
lib/CodeGen/RegisterCoalescer.cpp
Index: lib/CodeGen/RegisterCoalescer.cpp
===================================================================
--- lib/CodeGen/RegisterCoalescer.cpp
+++ lib/CodeGen/RegisterCoalescer.cpp
@@ -999,6 +999,38 @@
updateRegDefsUses(DstReg, DstReg, DstIdx);
NewMI->getOperand(0).setSubReg(NewIdx);
+ // Add dead subregister definitions if we are defining the whole register
+ // but only part of it is live.
+ // This could happen if the rematerialization instruction is rematerializing
+ // more than actually is used in the register.
+ // An example would be:
+ // vreg1 = LOAD CONSTANTS 5, 8 ; Loading both 5 and 8 in different subregs
+ // ; Copying only part of the register here, but the rest is undef.
+ // vreg2:sub_16bit<def, read-undef> = COPY vreg1:sub_16bit
+ // ==>
+ // ; Materialize all the constants but only using one
+ // vreg2 = LOAD_CONSTANTS 5, 8
+ //
+ // at this point for the part that wasn't defined before we could have
+ // subranges missing the definition.
+ if (MRI->shouldTrackSubRegLiveness(DstReg)) {
+ LiveInterval &DstInt = LIS->getInterval(DstReg);
+ if (NewIdx == 0 && DstInt.hasSubRanges()) {
+ SlotIndex CurrIdx = LIS->getInstructionIndex(NewMI);
+ unsigned MaxMask = MRI->getMaxLaneMaskForVReg(DstReg);
+ for (auto &SR : DstInt.subranges()) {
+ if (!SR.liveAt(CurrIdx.getRegSlot())) {
+ SR.createDeadDef(CurrIdx.getRegSlot(), LIS->getVNInfoAllocator());
+ }
+ MaxMask &= ~SR.LaneMask;
+ }
+ if (MaxMask != 0) {
+ LiveInterval::SubRange *SR =
+ DstInt.createSubRange(LIS->getVNInfoAllocator(), MaxMask);
+ SR->createDeadDef(CurrIdx.getRegSlot(), LIS->getVNInfoAllocator());
+ }
+ }
+ }
} else if (NewMI->getOperand(0).getReg() != CopyDstReg) {
// The New instruction may be defining a sub-register of what's actually
// been asked for. If so it must implicitly define the whole thing.
Index: lib/CodeGen/MachineVerifier.cpp
===================================================================
--- lib/CodeGen/MachineVerifier.cpp
+++ lib/CodeGen/MachineVerifier.cpp
@@ -1713,6 +1713,32 @@
LaneBitmask Mask = 0;
LaneBitmask MaxMask = MRI->getMaxLaneMaskForVReg(Reg);
+ // If the LiveInterval has subregister information we want to check that
+ // for each definition that defines the whole register every subrange has at
+ // least
+ // a dead definition for it.
+ if (LI.hasSubRanges()) {
+ for (auto DI = MRI->def_begin(Reg), DE = MachineRegisterInfo::def_end();
+ DI != DE; ++DI) {
+ MachineOperand &MO = *DI;
+ // We are defining the whole register, so all the subranges should have
+ // at least a dead definition.
+ if (MO.getSubReg() == 0) {
+ unsigned LiveMask = 0;
+ for (auto &SR : LI.subranges()) {
+ SlotIndex DefSlot =
+ LiveInts->getInstructionIndex(MO.getParent()).getRegSlot();
+ if (SR.liveAt(DefSlot))
+ LiveMask |= SR.LaneMask;
+ }
+ // If any lane didn't have a proper definition report.
+ if (LiveMask != MaxMask)
+ report("Lane mask defined but no sub range reports its definition",
+ MF);
+ }
+ }
+ }
+
for (const LiveInterval::SubRange &SR : LI.subranges()) {
if ((Mask & SR.LaneMask) != 0) {
report("Lane masks of sub ranges overlap in live interval", MF);
</pre>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<br>
<pre wrap="">_______________________________________________
llvm-commits mailing list
<a class="moz-txt-link-abbreviated" href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>
<a class="moz-txt-link-freetext" href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a>
</pre>
</blockquote>
<br>
This problem looks similar to one I was recently trying to debug on
this testcase. The problem seems to show up when a constant is
rematerialized into a subregister, but there is a remaining undef
component to the full register, causing an error with
-verify-machineinstrs and a scheduler crash without. However, this
patch seems to not solve it. Do you think this is a similar case you
may also have a problem with?<br>
<br>
-Matt<br>
</body>
</html>