[llvm] r262767 - RegisterCoalescer: Need to check DstReg+SrcReg for missing undef flags
Matthias Braun via llvm-commits
llvm-commits at lists.llvm.org
Thu Mar 17 11:56:07 PDT 2016
Hi,
> On Mar 17, 2016, at 10:42 AM, Nicolai Hähnle-Montoro <nhaehnle at gmail.com> wrote:
>
> Hi Matthias,
>
> this commit causes a regression in the AMDGPU backend. The attached
> shader, which is extracted from
> https://bugs.freedesktop.org/show_bug.cgi?id=94445, crashes:
>
> $ llc -march=amdgcn < ~/amd/bugs/fdo94445/shader.ll
> .text
> llc: /home/nha/amd/llvm/llvm/lib/CodeGen/MachineScheduler.cpp:1046:
> void llvm::ScheduleDAGMILive::updatePressureDiffs(llvm::ArrayRef<llvm::RegisterMaskPair>):
> Assertion `VNI && "No live value at use."' failed.
> #0 0x00007f8e0f6c54a2 llvm::sys::PrintStackTrace(llvm::raw_ostream&)
> /home/nha/amd/llvm/llvm/lib/Support/Unix/Signals.inc:322:0
> #1 0x00007f8e0f6c57fa PrintStackTraceSignalHandler(void*)
> /home/nha/amd/llvm/llvm/lib/Support/Unix/Signals.inc:380:0
> #2 0x00007f8e0f6c3d31 llvm::sys::RunSignalHandlers()
> /home/nha/amd/llvm/llvm/lib/Support/Signals.cpp:44:0
> #3 0x00007f8e0f6c4ebe SignalHandler(int)
> /home/nha/amd/llvm/llvm/lib/Support/Unix/Signals.inc:210:0
> #4 0x00007f8e0dc8e2f0 (/lib/x86_64-linux-gnu/libc.so.6+0x352f0)
> #5 0x00007f8e0dc8e267 gsignal
> /build/glibc-ryFjv0/glibc-2.21/signal/../sysdeps/unix/sysv/linux/raise.c:55:0
> #6 0x00007f8e0dc8feca abort /build/glibc-ryFjv0/glibc-2.21/stdlib/abort.c:91:0
> #7 0x00007f8e0dc8703d __assert_fail_base
> /build/glibc-ryFjv0/glibc-2.21/assert/assert.c:92:0
> #8 0x00007f8e0dc870f2 (/lib/x86_64-linux-gnu/libc.so.6+0x2e0f2)
> #9 0x00007f8e0faf5d3e
> llvm::ScheduleDAGMILive::updatePressureDiffs(llvm::ArrayRef<llvm::RegisterMaskPair>)
> /home/nha/amd/llvm/llvm/lib/CodeGen/MachineScheduler.cpp:1048:0
> #10 0x00007f8e0faf4e6d llvm::ScheduleDAGMILive::initRegPressure()
> /home/nha/amd/llvm/llvm/lib/CodeGen/MachineScheduler.cpp:932:0
> #11 0x00007f8e0faf692d
> llvm::ScheduleDAGMILive::buildDAGWithRegPressure()
> /home/nha/amd/llvm/llvm/lib/CodeGen/MachineScheduler.cpp:1177:0
> #12 0x00007f8e0faf60e9 llvm::ScheduleDAGMILive::schedule()
> /home/nha/amd/llvm/llvm/lib/CodeGen/MachineScheduler.cpp:1086:0
> #13 0x00007f8e0faf2ed8 (anonymous
> namespace)::MachineSchedulerBase::scheduleRegions(llvm::ScheduleDAGInstrs&,
> bool) /home/nha/amd/llvm/llvm/lib/CodeGen/MachineScheduler.cpp:493:0
> #14 0x00007f8e0faf21b7 (anonymous
> namespace)::MachineScheduler::runOnMachineFunction(llvm::MachineFunction&)
> /home/nha/amd/llvm/llvm/lib/CodeGen/MachineScheduler.cpp:353:0
> #15 0x00007f8e0fa9e267
> llvm::MachineFunctionPass::runOnFunction(llvm::Function&)
> /home/nha/amd/llvm/llvm/lib/CodeGen/MachineFunctionPass.cpp:44:0
> #16 0x00007f8e0f84b730
> llvm::FPPassManager::runOnFunction(llvm::Function&)
> /home/nha/amd/llvm/llvm/lib/IR/LegacyPassManager.cpp:1550:0
> #17 0x00007f8e0f84b8c9 llvm::FPPassManager::runOnModule(llvm::Module&)
> /home/nha/amd/llvm/llvm/lib/IR/LegacyPassManager.cpp:1571:0
> #18 0x00007f8e0f84bc44 (anonymous
> namespace)::MPPassManager::runOnModule(llvm::Module&)
> /home/nha/amd/llvm/llvm/lib/IR/LegacyPassManager.cpp:1627:0
> #19 0x00007f8e0f84c359
> llvm::legacy::PassManagerImpl::run(llvm::Module&)
> /home/nha/amd/llvm/llvm/lib/IR/LegacyPassManager.cpp:1730:0
> #20 0x00007f8e0f84c565 llvm::legacy::PassManager::run(llvm::Module&)
> /home/nha/amd/llvm/llvm/lib/IR/LegacyPassManager.cpp:1762:0
> #21 0x0000000000423cae compileModule(char**, llvm::LLVMContext&)
> /home/nha/amd/llvm/llvm/tools/llc/llc.cpp:408:0
> #22 0x0000000000422a72 main /home/nha/amd/llvm/llvm/tools/llc/llc.cpp:211:0
> #23 0x00007f8e0dc79a40 __libc_start_main
> /build/glibc-ryFjv0/glibc-2.21/csu/libc-start.c:323:0
> #24 0x0000000000421199 _start (/home/nha/amd/build-dbg/llvm/bin/llc+0x421199)
>
> Any idea as to what might cause this?
>
> Interestingly, a bugpoint-reduced variant crashes with a different
> assertion - so it's possible that there is a different bug in the
> backend which was exposed by your change, or it's just bugpoint
> violating some assumption. In any case, your input would be much
> appreciated.
the commit itself is fine. It looks like the scheduledag builder wrongly marks some subranges as live-out of the function. I'll look into it this afternoon.
I usually bugpoint with something like this to avoid it drifting to unrelated problems:
> bugpoint -compile-custom -compile-command ./run_llc.sh XXX.ll
with run_llc.sh:
if ! $HOME/dev/llvm/build/bin/llc -verify-machineinstrs "$@" 2>&1 | grep "The assert message I expected"; then
exit 0
else
exit $?
fi
Anyway this test is small enough for me to go on.
- Matthias
>
> Thanks,
> Nicolai
>
> On Fri, Mar 4, 2016 at 11:36 PM, Matthias Braun via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
>> Author: matze
>> Date: Fri Mar 4 22:36:10 2016
>> New Revision: 262767
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=262767&view=rev
>> Log:
>> RegisterCoalescer: Need to check DstReg+SrcReg for missing undef flags
>>
>> copy coalescing with enabled subregister liveness can reveal undef uses,
>> previously this was only checked for the SrcReg in updateRegDefsUses()
>> but we need to check DstReg as well.
>>
>> Modified:
>> llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp
>>
>> Modified: llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp?rev=262767&r1=262766&r2=262767&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp Fri Mar 4 22:36:10 2016
>> @@ -203,6 +203,16 @@ namespace {
>> /// make sure to set it to the correct physical subregister.
>> void updateRegDefsUses(unsigned SrcReg, unsigned DstReg, unsigned SubIdx);
>>
>> + /// If the given machine operand reads only undefined lanes add an undef
>> + /// flag.
>> + /// This can happen when undef uses were previously concealed by a copy
>> + /// which we coalesced. Example:
>> + /// %vreg0:sub0<def,read-undef> = ...
>> + /// %vreg1 = COPY %vreg0 <-- Coalescing COPY reveals undef
>> + /// = use %vreg1:sub1 <-- hidden undef use
>> + void addUndefFlag(const LiveInterval &Int, SlotIndex UseIdx,
>> + MachineOperand &MO, unsigned SubRegIdx);
>> +
>> /// Handle copies of undef values.
>> /// Returns true if @p CopyMI was a copy of an undef value and eliminated.
>> bool eliminateUndefCopy(MachineInstr *CopyMI);
>> @@ -1191,12 +1201,51 @@ bool RegisterCoalescer::eliminateUndefCo
>> return true;
>> }
>>
>> +void RegisterCoalescer::addUndefFlag(const LiveInterval &Int, SlotIndex UseIdx,
>> + MachineOperand &MO, unsigned SubRegIdx) {
>> + LaneBitmask Mask = TRI->getSubRegIndexLaneMask(SubRegIdx);
>> + if (MO.isDef())
>> + Mask = ~Mask;
>> + bool IsUndef = true;
>> + for (const LiveInterval::SubRange &S : Int.subranges()) {
>> + if ((S.LaneMask & Mask) == 0)
>> + continue;
>> + if (S.liveAt(UseIdx)) {
>> + IsUndef = false;
>> + break;
>> + }
>> + }
>> + if (IsUndef) {
>> + MO.setIsUndef(true);
>> + // We found out some subregister use is actually reading an undefined
>> + // value. In some cases the whole vreg has become undefined at this
>> + // point so we have to potentially shrink the main range if the
>> + // use was ending a live segment there.
>> + LiveQueryResult Q = Int.Query(UseIdx);
>> + if (Q.valueOut() == nullptr)
>> + ShrinkMainRange = true;
>> + }
>> +}
>> +
>> void RegisterCoalescer::updateRegDefsUses(unsigned SrcReg,
>> unsigned DstReg,
>> unsigned SubIdx) {
>> bool DstIsPhys = TargetRegisterInfo::isPhysicalRegister(DstReg);
>> LiveInterval *DstInt = DstIsPhys ? nullptr : &LIS->getInterval(DstReg);
>>
>> + if (DstInt && DstInt->hasSubRanges() && DstReg != SrcReg) {
>> + for (MachineOperand &MO : MRI->reg_operands(DstReg)) {
>> + unsigned SubReg = MO.getSubReg();
>> + if (SubReg == 0 || MO.isUndef())
>> + continue;
>> + MachineInstr &MI = *MO.getParent();
>> + if (MI.isDebugValue())
>> + continue;
>> + SlotIndex UseIdx = LIS->getInstructionIndex(MI).getRegSlot(true);
>> + addUndefFlag(*DstInt, UseIdx, MO, SubReg);
>> + }
>> + }
>> +
>> SmallPtrSet<MachineInstr*, 8> Visited;
>> for (MachineRegisterInfo::reg_instr_iterator
>> I = MRI->reg_instr_begin(SrcReg), E = MRI->reg_instr_end();
>> @@ -1238,30 +1287,11 @@ void RegisterCoalescer::updateRegDefsUse
>> LaneBitmask Mask = MRI->getMaxLaneMaskForVReg(DstInt->reg);
>> DstInt->createSubRangeFrom(Allocator, Mask, *DstInt);
>> }
>> - LaneBitmask Mask = TRI->getSubRegIndexLaneMask(SubIdx);
>> - bool IsUndef = true;
>> SlotIndex MIIdx = UseMI->isDebugValue()
>> ? LIS->getSlotIndexes()->getIndexBefore(*UseMI)
>> : LIS->getInstructionIndex(*UseMI);
>> SlotIndex UseIdx = MIIdx.getRegSlot(true);
>> - for (LiveInterval::SubRange &S : DstInt->subranges()) {
>> - if ((S.LaneMask & Mask) == 0)
>> - continue;
>> - if (S.liveAt(UseIdx)) {
>> - IsUndef = false;
>> - break;
>> - }
>> - }
>> - if (IsUndef) {
>> - MO.setIsUndef(true);
>> - // We found out some subregister use is actually reading an undefined
>> - // value. In some cases the whole vreg has become undefined at this
>> - // point so we have to potentially shrink the main range if the
>> - // use was ending a live segment there.
>> - LiveQueryResult Q = DstInt->Query(MIIdx);
>> - if (Q.valueOut() == nullptr)
>> - ShrinkMainRange = true;
>> - }
>> + addUndefFlag(*DstInt, UseIdx, MO, SubIdx);
>> }
>>
>> if (DstIsPhys)
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
>
> --
> Lerne, wie die Welt wirklich ist,
> aber vergiss niemals, wie sie sein sollte.
> <shader.ll>
More information about the llvm-commits
mailing list