search

Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

[SystemZ + SubregLiveness] Assertion `!NodePtr->isKnownSentinel()' failed. #38249

Closed Quuxplusone closed 5 years ago

Quuxplusone commented 6 years ago
Bugzilla Link PR39276
Status RESOLVED FIXED
Importance P enhancement
Reported by Jonas Paulsson (paulsson@linux.vnet.ibm.com)
Reported on 2018-10-13 02:02:18 -0700
Last modified on 2018-11-12 00:50:37 -0800
Version trunk
Hardware PC Linux
CC dimitry@andric.com, kparzysz@quicinc.com, llvm-bugs@lists.llvm.org, paulsson@linux.vnet.ibm.com, uweigand@de.ibm.com
Fixed by commit(s)
Attachments tc-subregliven-coal.ll (2055 bytes, text/plain)
crash2.i (203938 bytes, text/plain)
Blocks
Blocked by
See also 
Created attachment 21000
reduced testcase

llc -mtriple=s390x-linux-gnu -mcpu=z13 ./tc-subregliven-coal.ll -systemz-subreg-
liveness -disable-machine-dce

clang-8: /home/ijonpan/llvm/llvm-dev/include/llvm/ADT/ilist_iterator.h:140:
Assertion `!NodePtr->isKnownSentinel()' failed.

#8 0x000002aa30e16ed0 llvm::SystemZRegisterInfo::shouldCoalesce
#9 0x000002aa3119dd8c (anonymous namespace)::RegisterCoalescer::joinCopy
Quuxplusone commented 6 years ago

Attached tc-subregliven-coal.ll (2055 bytes, text/plain): reduced testcase

Quuxplusone commented 6 years ago

Attached crash2.i (203938 bytes, text/plain): clang unreduced test case (csmith)

Quuxplusone commented 5 years ago

Jonas, can you please check if https://reviews.llvm.org/rL330345 (fix for bug 36825) fixes this issue for you?

Quuxplusone commented 5 years ago 
(In reply to Dimitry Andric from comment #2)
> Jonas, can you please check if https://reviews.llvm.org/rL330345 (fix for
> bug 36825) fixes this issue for you?

I am not sure what to do here - r330345 seems to have been committed already in
April, and it seems to not have been reverted... And I can not revert it
cleanly, if that's what you meant?
Quuxplusone commented 5 years ago 
(In reply to Jonas Paulsson from comment #3)
> (In reply to Dimitry Andric from comment #2)
> > Jonas, can you please check if https://reviews.llvm.org/rL330345 (fix for
> > bug 36825) fixes this issue for you?
>
> I am not sure what to do here - r330345 seems to have been committed already
> in April, and it seems to not have been reverted... And I can not revert it
> cleanly, if that's what you meant?

Ah sorry, I didn't realize that you had tested with a newer version already.
Indeed, your test case still crashes with trunk r346070:

$ /home/dim/obj/llvm-trunk-r346070/bin/clang-8 -cc1 -triple systemz -emit-obj -
disable-free -main-file-name crash2.i -mrelocation-model static -mthread-model
posix -mdisable-fp-elim -fmath-errno -masm-verbose -mconstructor-aliases -
target-cpu z13 -dwarf-column-info -debugger-tuning=gdb -resource-dir
/home/dim/obj/llvm-trunk-r346070/lib/clang/8.0.0 -O3 -w -fdebug-compilation-dir
/home/dim/bugs/pr39276 -ferror-limit 19 -fmessage-length 227 -fno-signed-char -
fobjc-runtime=gcc -fdiagnostics-show-option -fcolor-diagnostics -vectorize-
loops -vectorize-slp -mllvm -disable-machine-dce -mllvm -systemz-subreg-
liveness -mllvm -disable-basicaa -o /tmp/crash2-eec497.o -x cpp-output crash2.i
-faddrsig
Assertion failed: (!NodePtr->isKnownSentinel()), function operator*, file
/home/dim/src/llvm/trunk/include/llvm/ADT/ilist_iterator.h, line 140.
Stack dump:
0.      Program arguments: /home/dim/obj/llvm-trunk-r346070/bin/clang-8 -cc1 -
triple systemz -emit-obj -disable-free -main-file-name crash2.i -mrelocation-
model static -mthread-model posix -mdisable-fp-elim -fmath-errno -masm-verbose -
mconstructor-aliases -target-cpu z13 -dwarf-column-info -debugger-tuning=gdb -
resource-dir /home/dim/obj/llvm-trunk-r346070/lib/clang/8.0.0 -O3 -w -fdebug-
compilation-dir /home/dim/bugs/pr39276 -ferror-limit 19 -fmessage-length 227 -
fno-signed-char -fobjc-runtime=gcc -fdiagnostics-show-option -fcolor-
diagnostics -vectorize-loops -vectorize-slp -mllvm -disable-machine-dce -mllvm -
systemz-subreg-liveness -mllvm -disable-basicaa -o /tmp/crash2-eec497.o -x cpp-
output crash2.i -faddrsig
1.      <eof> parser at end of file
2.      Code generation
3.      Running pass 'Function Pass Manager' on module 'crash2.c'.
4.      Running pass 'Simple Register Coalescing' on function '@main'
#0 0x0000000003232d28 llvm::sys::PrintStackTrace(llvm::raw_ostream&)
(/home/dim/obj/llvm-trunk-r346070/bin/clang-8+0x3232d28)
#1 0x0000000003230d28 llvm::sys::RunSignalHandlers() (/home/dim/obj/llvm-trunk-
r346070/bin/clang-8+0x3230d28)
#2 0x0000000003233348 SignalHandler(int) (/home/dim/obj/llvm-trunk-
r346070/bin/clang-8+0x3233348)
#3 0x000000080519f166 handle_signal /usr/src/lib/libthr/thread/thr_sig.c:0:3
Abort trap

so this must be a different issue than bug 36825.
Quuxplusone commented 5 years ago

The bug is in SystemZRegisterInfo::shouldCoalesce. It gets the instruction indexes wrong.

Quuxplusone commented 5 years ago 
(In reply to Krzysztof Parzyszek from comment #5)
> The bug is in SystemZRegisterInfo::shouldCoalesce. It gets the instruction
> indexes wrong.

SystemZRegisterInfo::shouldCoalesce() is aiming to only coalesce two (MBB)
local GR128 intervals and so returns false if either source or destination
interval returns true with LIS.isLiveOutOfMBB() or LIS.isLiveInToMBB().

After this, the region in MBB is traversed, by first finding the first and last
MIs from the two intervals. After the first live-in/live-out checks it is
assumed that FirstMI and LastMI is in MBB.

This crash occurred because LastMI is not in fact in the same MBB as FirstMI.
To me it looks like this is because the interval for %6 is corrupt, since the
live ranges considering bb2 are missing:

********** INTERVALS **********
%0 [288r,288d:0)  0@288r weight:0.000000e+00
%1 [16r,64r:0)  0@16r weight:0.000000e+00
%2 [32r,48r:0)  0@32r weight:0.000000e+00
%3 [48r,64r:0)[64r,128r:1)  0@48r 1@64r weight:0.000000e+00
%4 [80r,96r:0)  0@80r weight:0.000000e+00
%5 [96r,112r:0)  0@96r L00000009 [96r,112r:0)  0@96r weight:0.000000e+00
%6 [112r,128r:0)[128r,128d:1)  0@112r 1@128r weight:0.000000e+00
%7 [144r,160r:0)  0@144r weight:0.000000e+00
%8 [224r,240r:0)  0@224r weight:0.000000e+00
RegMasks:
********** MACHINEINSTRS **********
# Machine code for function main: NoPHIs, TracksLiveness

0B      bb.0 (%ir-block.0):
          successors: %bb.2(0x00000001), %bb.1(0x7fffffff); %bb.2(0.00%), %bb.1(100.00%)

16B       %1:addr64bit = LARL @g_193
32B       %2:gr32bit = IIFMux 2899813578
48B       %3:gr32bit = COPY %2:gr32bit
64B       %3:gr32bit = O %3:gr32bit(tied-def 0), %1:addr64bit, 0, $noreg,
implicit-def dead $cc :: (dereferenceable load 4 from @g_193, !tbaa !1)
80B       %4:gr64bit = LGFI -1395153718
96B       undef %5.subreg_l64:gr128bit = COPY %4:gr64bit
112B      %6:gr128bit = COPY %5:gr128bit
128B      dead %6:gr128bit = DSGFR %6:gr128bit(tied-def 0), %3:gr32bit
144B      %7:grx32bit = LHIMux 0
160B      CHIMux %7:grx32bit, 0, implicit-def $cc
176B      BRC 14, 6, %bb.2, implicit killed $cc
192B      J %bb.1

208B    bb.1 (%ir-block.4):
        ; predecessors: %bb.0

224B      %8:gr32bit = LHIMux -9
240B      STRL %8:gr32bit, @g_74 :: (store 4 into @g_74, !tbaa !1)
256B      Return

272B    bb.2 (%ir-block.5):
        ; predecessors: %bb.0

288B      dead %0:grx32bit = COPY undef %6.subreg_l32:gr128bit

# End machine code for function main.

This is not the case without -systemz-subreg-liveness:

********** INTERVALS **********
%0 [288r,288d:0)  0@288r weight:0.000000e+00
%1 [16r,64r:0)  0@16r weight:0.000000e+00
%2 [32r,48r:0)  0@32r weight:0.000000e+00
%3 [48r,64r:0)[64r,128r:1)  0@48r 1@64r weight:0.000000e+00
%4 [80r,96r:0)  0@80r weight:0.000000e+00
%5 [96r,112r:0)  0@96r weight:0.000000e+00
%6 [112r,128r:0)[128r,208B:1)[272B,288r:1)  0@112r 1@128r weight:0.000000e+00
%7 [144r,160r:0)  0@144r weight:0.000000e+00
%8 [224r,240r:0)  0@224r weight:0.000000e+00
RegMasks:
********** MACHINEINSTRS **********
# Machine code for function main: NoPHIs, TracksLiveness

0B      bb.0 (%ir-block.0):
          successors: %bb.2(0x00000001), %bb.1(0x7fffffff); %bb.2(0.00%), %bb.1(100.00%)

16B       %1:addr64bit = LARL @g_193
32B       %2:gr32bit = IIFMux 2899813578
48B       %3:gr32bit = COPY %2:gr32bit
64B       %3:gr32bit = O %3:gr32bit(tied-def 0), %1:addr64bit, 0, $noreg,
implicit-def dead $cc :: (dereferenceable load 4 from @g_193, !tbaa !1)
80B       %4:gr64bit = LGFI -1395153718
96B       undef %5.subreg_l64:gr128bit = COPY %4:gr64bit
112B      %6:gr128bit = COPY %5:gr128bit
128B      %6:gr128bit = DSGFR %6:gr128bit(tied-def 0), %3:gr32bit
144B      %7:grx32bit = LHIMux 0
160B      CHIMux %7:grx32bit, 0, implicit-def $cc
176B      BRC 14, 6, %bb.2, implicit killed $cc
192B      J %bb.1

208B    bb.1 (%ir-block.4):
        ; predecessors: %bb.0

224B      %8:gr32bit = LHIMux -9
240B      STRL %8:gr32bit, @g_74 :: (store 4 into @g_74, !tbaa !1)
256B      Return

272B    bb.2 (%ir-block.5):
        ; predecessors: %bb.0

288B      dead %0:grx32bit = COPY %6.subreg_l32:gr128bit

# End machine code for function main.
Quuxplusone commented 5 years ago

The use of %6 in bb.2 is "undef", and so it won't be reflected in live ranges. Look at the range of indexes that the loop in shouldCoalesce traverses---it starts in one block and ends in another. In the process it walks off the end of the first block and hits the assertion.

Quuxplusone commented 5 years ago 
(In reply to Krzysztof Parzyszek from comment #7)
> The use of %6 in bb.2 is "undef", and so it won't be reflected in live
> ranges.  Look at the range of indexes that the loop in shouldCoalesce
> traverses---it starts in one block and ends in another. In the process it
> walks off the end of the first block and hits the assertion.

Aha - so it's not supposed to be included in the live interval for %6.

It seems that it would be best to return 'true' for undef source operands, so
that RegisterCoalescer::joinCopy() will continue and call eliminateUndefCopy(),
right?
Quuxplusone commented 5 years ago 
Returning "true" should be ok, but what I'm wondering is whether
beginIndex/endIndex (that shouldCoalesce is calling) can end up in different
basic blocks despite the checks for live-in/out. Consider this:

1  bb.1:
2    dead %0 = ...

3  bb.2:
4    %0 = ...
5    ... = %0

The live range for %0 would be [2d,2d)[4r,5r), spanning two blocks, even though
%0 is not live-in/out from bb.2.

The proper solution really depends on what you want to do in such cases. If you
only want to ensure that both are local to MBB, maybe you could skip the
liveness checks, and simply compare the basic blocks for begin/endIndex. This
would reject the %0 above. If you still want to do coalescing, you could keep
the liveness checks, and instead of begin/endIndex, pick index range that falls
within bb.2.
Quuxplusone commented 5 years ago

Thanks!

Proposed patch at https://reviews.llvm.org/D54197.

Quuxplusone commented 5 years ago

r346406