[llvm] r262767 - RegisterCoalescer: Need to check DstReg+SrcReg for missing undef flags

Nicolai Hähnle-Montoro via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 23 13:12:23 PDT 2016


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.


More information about the llvm-commits mailing list