Mikael Holmén
2014-Aug-19 11:09 UTC
[LLVMdev] Help with definition of subregisters; spill, rematerialization and implicit uses
Hi Quentin, On 08/15/14 19:01, Quentin Colombet wrote: [...]>> The question is: How should true subregister definitions be >> expressed so that they do not interfere with each other? See the >> detailed problem description below. > > We do have a limitation in our current liveness tracking for > sub-register. Therefore, I am not sure that is possible. > > Conceptually, what you want is: > Reg5:hi16<def,read-undef> > Reg5:lo16<def,read-undef> > > However, I guess this wouldn’t be supported, because it would mean that > we do not care about the value of hi16 at the definition of Reg5:lo16. > This is true, but after this definition we do care about hi16 and I am > afraid read-undef does not convey the right information for the > subsequent uses of Reg5. > > You can give it a try and see how it goes.I tried setting isUndef to trie when handling INSERT_SUBREG in TwoAddressInstructioPass.cpp, but then I run into stuff like this instead: 832B %vreg50:hi16<def,read-undef> = COPY %vreg0 848B ... 864B %vreg19<def,dead> = COPY %vreg50 880B %vreg19:lo16<def,read-undef> = COPY %vreg73 896B ... 912B mv_a32_r16_rmod1 %vreg19, %vreg20 ... *** Bad machine code: Multiple connected components in live interval *** - function: fixedconv - interval: %vreg19 [864r,864d:0)[880r,1024r:1) 0 at 864r 1 at 880r 0: valnos 0 1: valnos 1 So here, both the setting of the hi16 and lo16 parts are marked with read-undef, as wanted. However 864B %vreg19<def,dead> = COPY %vreg50 looks suspicious to me. It's like it thinks that 880B %vreg19:lo16<def,read-undef> = COPY %vreg73 is redefining the whole vreg19, not only the lo16 part, and since this instruction has read-undef, it thinks no part of vreg19, not even hi16 is live over instruction 880.>> >> isUndef() will return false and getSubReg() true, and thus readsReg() >> true and the reload is inserted. >> >> Then we get >> >> *** Bad machine code: Instruction loads from dead spill slot *** >> >> because the spill slot has not been written. > > This is weird. Any chance you could share a test case?Unfortunately not. I'm running our out-of-tree backend and I've no idea if anything like this ever happens in other backends :( Thanks! /Mikael
Quentin Colombet
2014-Aug-19 16:58 UTC
[LLVMdev] Help with definition of subregisters; spill, rematerialization and implicit uses
Hi Mikael, On Aug 19, 2014, at 4:09 AM, Mikael Holmén <mikael.holmen at ericsson.com> wrote:> Hi Quentin, > > On 08/15/14 19:01, Quentin Colombet wrote: > [...] >>> The question is: How should true subregister definitions be >>> expressed so that they do not interfere with each other? See the >>> detailed problem description below. >> >> We do have a limitation in our current liveness tracking for >> sub-register. Therefore, I am not sure that is possible. >> >> Conceptually, what you want is: >> Reg5:hi16<def,read-undef> >> Reg5:lo16<def,read-undef> >> >> However, I guess this wouldn’t be supported, because it would mean that >> we do not care about the value of hi16 at the definition of Reg5:lo16. >> This is true, but after this definition we do care about hi16 and I am >> afraid read-undef does not convey the right information for the >> subsequent uses of Reg5. >> >> You can give it a try and see how it goes. > > I tried setting isUndef to trie when handling INSERT_SUBREG in TwoAddressInstructioPass.cpp, but then I run into stuff like this instead: > > 832B %vreg50:hi16<def,read-undef> = COPY %vreg0 > 848B ... > 864B %vreg19<def,dead> = COPY %vreg50 > 880B %vreg19:lo16<def,read-undef> = COPY %vreg73 > 896B ... > 912B mv_a32_r16_rmod1 %vreg19, %vreg20 > > ... > > *** Bad machine code: Multiple connected components in live interval *** > - function: fixedconv > - interval: %vreg19 [864r,864d:0)[880r,1024r:1) 0 at 864r 1 at 880r > 0: valnos 0 > 1: valnos 1 > > So here, both the setting of the hi16 and lo16 parts are marked with read-undef, as wanted. However > > 864B %vreg19<def,dead> = COPY %vreg50 > > looks suspicious to me. It's like it thinks that > > 880B %vreg19:lo16<def,read-undef> = COPY %vreg73 > > is redefining the whole vreg19, not only the lo16 part, and since this instruction has read-undef, it thinks no part of vreg19, not even hi16 is live over instruction 880.I was afraid something like this would happen. Your interpretation is correct. This confirms that the semantic of read-undef flag means that the others part of the register are undef, and thus nothing live through. The bottom line is currently you cannot represent what you want. It seems that you will have to debug further the *** Bad machine code: Instruction loads from dead spill slot *** before we can be of any help. Thanks, -Quentin> >>> >>> isUndef() will return false and getSubReg() true, and thus readsReg() >>> true and the reload is inserted. >>> >>> Then we get >>> >>> *** Bad machine code: Instruction loads from dead spill slot *** >>> >>> because the spill slot has not been written. >> >> This is weird. Any chance you could share a test case? > > Unfortunately not. I'm running our out-of-tree backend and I've no idea if anything like this ever happens in other backends :( > > Thanks! > /Mikael >
Mikael Holmén
2014-Aug-22 12:17 UTC
[LLVMdev] Help with definition of subregisters; spill, rematerialization and implicit uses
Hi Quentin, On 08/19/14 18:58, Quentin Colombet wrote: [...]> It seems that you will have to debug further the *** Bad machine code: Instruction loads from dead spill slot *** before we can be of any help.Yes, I've done some more digging. Sorry for the long mail... I get: Inline spilling aN40_0_7:%vreg1954 [5000r,5056r:0)[5056r,5348r:1) 0 at 5000r 1 at 5056r At this point I have the following live ranges for vreg1954: %vreg1954 [5000r,5056r:0)[5056r,5348r:1) 0 at 5000r 1 at 5056r And vreg1954 is mentioned in these instructions: 5000B %vreg1954:hi16<def,read-undef> = mv_any16 32766 [...] 5048B mv_ar16_r16_rmod1 %vreg1954:hi16, %vreg1753 5056B %vreg1954:lo16<def> = mv_nimm6_ar16 0 5064B Store40FI %vreg1954, <fi#2> [...] 5128B %vreg223<def> = COPY %vreg1954 [...] 5216B %vreg1178<def> = COPY %vreg1954 [...] 5348B %vreg1955<def> = COPY %vreg1954 Then it tries to rematerialize: Value %vreg1954:0 at 5000r may remat from %vreg1954:hi16<def,read-undef> = mv_any16 32766 And it examines all use points of vreg1954 to see if it can remat or not: cannot remat for 5128e %vreg223<def> = COPY %vreg1954 cannot remat for 5216e %vreg1178<def> = COPY %vreg1954 remat: 5044r %vreg1956:hi16<def,read-undef> = mv_any16 32766 5048e mv_ar16_r16_rmod1 %vreg1956:hi16<kill>, %vreg1753 cannot remat for 5064e Store40FI %vreg1954, <fi#2> cannot remat for 5348e %vreg1955<def> = COPY %vreg1954 All defs dead: %vreg1954:hi16<def,read-undef,dead> = mv_any16 32766 Remat created 1 dead defs. Deleting dead def 5000r %vreg1954:hi16<def,read-undef,dead> = mv_any16 32766 1 registers to spill after remat. What strikes me here is that it never mentions the instruction 5056B %vreg1954:lo16<def> = mv_nimm6_ar16 0 where we do have a read of vreg1954:hi16 since there is no read-undef on the def operand. Is this how it's intended to be? (The use points are found by for (MachineRegisterInfo::use_bundle_nodbg_iterator RI = MRI.use_bundle_nodbg_begin(Reg), E = MRI.use_bundle_nodbg_end(); RI != E; ) { anyRemat |= reMaterializeFor(LI, MI); } and MachineRegisterInfo::defusechain_instr_iterator::advance seems to skip all def operands for use_bundle_nodbg_iterator since ReturnDefs is false. So it will happily advance past the instruction setting lo16.) Then after the remats it does spillAroundUses %vreg1954 and there I get reload: 5052r %vreg1957<def> = Load40FI <fi#2> rewrite: 5056r %vreg1957:lo16<def> = mv_nimm6_ar16 0 since no remat was inserted before 5056. But noone has stored anything at fi#2 so I end up with *** Bad machine code: Instruction loads from dead spill slot *** Does everything above, except the error at the end, look ok to you? I tried fiddling with MachineRegisterInfo::defusechain_instr_iterator to make it also return the %vreg1954:lo16<def> = mv_nimm6_ar16 0 instruction, and then my program actually compiled succesfully! My diff: @@ -886,11 +886,11 @@ public: explicit defusechain_instr_iterator(MachineOperand *op) : Op(op) { // If the first node isn't one we're interested in, advance to one that // we are interested in. if (op) { if ((!ReturnUses && op->isUse()) || - (!ReturnDefs && op->isDef()) || + (!ReturnDefs && op->isDef() && !op->readsReg()) || (SkipDebug && op->isDebug())) advance(); } } friend class MachineRegisterInfo; @@ -907,11 +907,11 @@ public: else assert(!Op->isDebug() && "Can't have debug defs"); } } else { // If this is an operand we don't care about, skip it. - while (Op && ((!ReturnDefs && Op->isDef()) || + while (Op && ((!ReturnDefs && Op->isDef() && !Op->readsReg()) || (SkipDebug && Op->isDebug()))) Op = getNextOperandForReg(Op); } } public: But of course then other tests fail. For example: build-all/./bin/llc < test/CodeGen/R600/literals.ll -march=r600 -mcpu=redwood gives llc: ../lib/CodeGen/TwoAddressInstructionPass.cpp:684: void (anonymous namespace)::TwoAddressInstructionPass::scanUses(unsigned int): Assertion `SrcRegMap[NewReg] == Reg && "Can't map to two src registers!"' failed. So I suppose there are assumptions that defusechain_instr_iterator ignores implicit sub register use when defining some sub register. :/ What's your thoughts on this? Thanks, Mikael