[llvm] r262767 - RegisterCoalescer: Need to check DstReg+SrcReg for missing undef flags
Matthias Braun via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 23 13:22:39 PDT 2016
Hi Nicolai,
the issue is a problem that existed even before that patch that involves what I call "hidden" dead defs now. With subregsiters you can have cases like this:
%vreg0:sub1<dead> = some definition
%vreg1 = COPY %vreg0
= use %vreg1:sub0
Which really is a dead definition and a use of an undefined value, but they are hidden by the COPY and not obviously visible by the usual SSA analyses because they only affect some subregisters. The RegisterCoalescer code does not expect values to suddenly become dead after removing a COPY. There are various workarounds in the register coalescer today but they are not enough as shown again.
The proper fix I am working on now introduces an earlier pass which performs a dataflow analysis over COPY instructions to determine these dead and unused operands upfront so they get handled correctly by liveness calculation etc.
Anyway to move on: It may be a bit longer as writing a whole pass takes time, feel free to revert r262767 in the meantime.
- Matthias
> On Mar 23, 2016, at 1:12 PM, Nicolai Hähnle-Montoro via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>
> Hi Matthias,
>
> do you have any news about this?
>
> On Thu, Mar 17, 2016 at 1:56 PM, Matthias Braun <matze at braunis.de> wrote:
>> 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>
>>
>
>
>
> --
> Lerne, wie die Welt wirklich ist,
> aber vergiss niemals, wie sie sein sollte.
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list