There’s an assert at line 192, lib/CodeGen/RegisterScavenging.cpp that appears to get tripped on targets that don’t have subregisters defined:
bool SubUsed = false;
for (const unsigned *SubRegs = TRI->getSubRegisters(Reg);
unsigned SubReg = *SubRegs; ++SubRegs)
if (isUsed(SubReg)) {
SubUsed = true;
break;
}
assert(SubUsed && “Using an undefined register!”);
CellSPU does not define any subregisters; consequently, SubUsed will always be false and trip the assert. What’s the intended behavior (before I submit a bug report)?
-scooter
Ugh. Management lobotomy kicked in. Need to RTFC better.
No, I wasn’t having a management lobotomy moment. If the target’s registers have no subregisters, SubUsed is false and the assert gets tripped.
Ok, back to the original question: What was the original intent in this code (lines 186-193 in lib/CodeGen/RegisterScavenging.cpp)?
-scooter
Scott Michel skrev:
No, I wasn't having a management lobotomy moment. If the target's registers have
no subregisters, SubUsed is false and the assert gets tripped.
Ok, back to the original question: What was the original intent in this code
(lines 186-193 in lib/CodeGen/RegisterScavenging.cpp)?
You beat me to it :). A simple bypass (patch attached) does at least not
break 'make check' with x86 and SPU backends. (And fixes the problem
with call.ll-test breaking when the SPU bigframes patch is applied)
kalle
regscavenge_subreg.patch (853 Bytes)
Kalle:
Your patch is similar to what I’d coded (and am testing, which means a couple of hours before I consider committing). Other than cosmetic changes and changing ‘NULL’ to ‘0’ (it’s an integer list, after all). This patch now causes new problems in the CellSPU backend (more stqd’s and lqd’s), so I have to investigate those before committing the patch.
‘make test’ is our friend. ‘make test’ is our friend. ‘make test’ is our friend. 
-scooter
I am afraid the assert is correct, you are ignoring the outer "if (!isUsed(Reg))..."
Without sub-registers, the assert would simplify to:
if (MO.isUse()) {
assert(isUsed(Reg) && "Using an undefined register!")
} else {
assert(MO.isDef());
llc -verify-machineinstrs will probably tell you the same thing.
My guess is that CellSPU is not properly managing kill flags on machine operands. That is a requirement before you can use the register scavenger. It took a while to get ARM to that state.
/jakob
Jakob Stoklund Olesen skrev:
This patch now causes new problems in the CellSPU
backend (more stqd's and lqd's), so I have to investigate those
before committing the patch.
Yes, it is because the register scavenger unconditionally allocates a
spill slot from the stack, thus eliminating the possibility of a small
prologue and epilogue.
If you look at the ARM target, you will see multiple solutions.
- Thumb1 uses a fixed, reserved scratch register (r12) instead of the scavenger.
- If possible, an extra callee-saved register is spilled instead of allocating an emergency spill slot.
- Finally, an emergency spill slot is only allocated if it is needed.
I am afraid the assert is correct, you are ignoring the outer "if
(!isUsed(Reg))..."
Hmm.. didn't miss it. Just didn't understand it
So the problem is that a use-operation does not have its register marked
as used? I.e. something is badly broken?
Exactly. If a register is not marked as used, the scavenger thinks it is free to be used as a scratch register. That is very bad if it was really needed.
It means that the backend is using a register after it had been <kill>ed. (Or maybe before it was <def>ined).
Jakob Stoklund Olesen skrev:
This patch now causes new problems in the CellSPU
backend (more stqd's and lqd's), so I have to investigate those
before committing the patch.
Yes, it is because the register scavenger unconditionally allocates a
spill slot from the stack, thus eliminating the possibility of a small
prologue and epilogue.
If you look at the ARM target, you will see multiple solutions.
- Thumb1 uses a fixed, reserved scratch register (r12) instead of the scavenger.
<nitpick>
ARM mode used to reserve R12 for this. Thumb used to reserve R3. Both ARM and Thumb now use the scavenger.
</nitpick>