Abort trap in 'PowerPC DAG->DAG Pattern Instruction Selection'

Quuxplusone commented 6 years ago


Bugzilla Link	PR36100
Status	CONFIRMED
Importance	P normal
Reported by	Sven Siemsen (sven@sven-siemsen.de)
Reported on	2018-01-25 14:32:45 -0800
Last modified on	2020-08-31 00:34:33 -0700
Version	trunk
Hardware	Other FreeBSD
CC	chmeeedalf@gmail.com, fwage73@gmail.com, hfinkel@anl.gov, james@jedavies.org, llvm-bugs@lists.llvm.org, nemanja.i.ibm@gmail.com, powerllvm@ca.ibm.com
Fixed by commit(s)
Attachments	`progressmeter-2c5082.zip` (331560 bytes, application/zip) `reproducer.ll` (7435 bytes, text/plain)
Blocks
Blocked by
See also

Created attachment 19752
Preprocessed source and associated run script

I am trying to build FreeBSD-11.1 on powerpc64 with clang-5.0.1 from ports.
When building openssh, clang crashes

--- progressmeter.o ---
clang-5.0: error: unable to execute command: Abort trap (core dumped)
clang-5.0: error: clang frontend command failed due to signal (use -v to see
invocation)
clang version 5.0.1 (tags/RELEASE_501/final)
Target: powerpc-portbld-freebsd11.1
Thread model: posix
InstalledDir: /usr/local/llvm50/bin
clang-5.0: note: diagnostic msg: PLEASE submit a bug report to
http://llvm.org/bugs/ and include the crash backtrace, preprocessed source, and
associated run script.
clang-5.0: note: diagnostic msg:
PLEASE ATTACH THE FOLLOWING FILES TO THE BUG REPORT:
Preprocessed source(s) and associated run script(s) are located at:
clang-5.0: note: diagnostic msg: /tmp/progressmeter-2c5082.c
clang-5.0: note: diagnostic msg: /tmp/progressmeter-2c5082.sh

Output of progressmeter-2c5082.sh:

Assertion failed: (NewLHS.getValueType() == N->getValueType(0) && "Unexpected
setcc expansion!"), function ExpandIntOp_SETCC, file
/usr/ports/devel/llvm50/work/llvm-
5.0.1.src/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp, line 3117.
Stack dump:
0.      Program arguments: /usr/local/llvm50/bin/clang-5.0 -cc1 -triple powerpc-
portbld-freebsd11.1 -emit-obj -disable-free -main-file-name progressmeter.c -
mrelocation-model static -mthread-model posix -mdisable-fp-elim -masm-verbose -
mconstructor-aliases -target-cpu 970 -mfloat-abi hard -dwarf-column-info -
debugger-tuning=gdb -coverage-notes-file
/usr/obj/usr/src/world32/usr/src/secure/lib/libssh/progressmeter.gcno -sys-
header-deps -D COMPAT_32BIT -D HAVE_LDNS=1 -O2 -Wno-pointer-sign -Wno-empty-
body -Wno-string-plus-int -Wno-unused-const-variable -Wno-tautological-compare -
Wno-unused-value -Wno-parentheses-equality -Wno-unused-function -Wno-enum-
conversion -Wno-unused-local-typedef -Wno-address-of-packed-member -Wno-switch -
Wno-switch-enum -Wno-knr-promoted-parameter -Wno-parentheses -std=gnu99 -ferror-
limit 19 -fmessage-length 0 -stack-protector 2 -fno-signed-char -fobjc-
runtime=gnustep -fdiagnostics-show-option -vectorize-loops -vectorize-slp -x c
progressmeter-2c5082.c
1.      <eof> parser at end of file
2.      Code generation
3.      Running pass 'Function Pass Manager' on module 'progressmeter-2c5082.c'.
4.      Running pass 'PowerPC DAG->DAG Pattern Instruction Selection' on
function '@Fssh_refresh_progress_meter'
Abort trap (core dumped)

I tried with clang 7.0.0 from devel/clang-devel, same result.

Quuxplusone commented 6 years ago

Attached progressmeter-2c5082.zip (331560 bytes, application/zip): Preprocessed source and associated run script

Quuxplusone commented 6 years ago

This seems to be an issue with legalization somehow creating both i32 and i1
setcc nodes. I haven't dug into how this happens yet, but we create the
following node:

t21: i32 = setcc t19, Constant:i64<1>, setugt:ch

and when we call IntegerExpandSetCCOperands(), we end up with the new LHS being
a select with a different type (since we've created i1 setcc nodes):

    t47: i1 = setcc t42, Constant:i32<0>, seteq:ch
    t43: i1 = setcc t41, Constant:i32<1>, setugt:ch
    t45: i1 = setcc t42, Constant:i32<0>, setne:ch
  t48: i1 = select t47, t43, t45

In any case, this reproduces with the following minimal test case:

float test(long long a) {
  return a;
}

Compiled with:
clang -O2 --target=powerpc-portbld-freebsd11.1 a.c -S -mcpu=970

Quuxplusone commented 6 years ago

I think the problem is in our handling of int->float conversion. We
unconditionally emit i32 setcc nodes rather than asking TargetLowering what
result type to use. Since we use a different type depending on whether we've
got CRBits or not, it seems that we should actually be querying that when
emitting setcc nodes.

This certainly seems to fix the problem and is likely the right thing to do
(although incomplete as there are other places we unconditionally emit an i32
setcc) - what do you think Hal:
Index: lib/Target/PowerPC/PPCISelLowering.cpp
===================================================================
--- lib/Target/PowerPC/PPCISelLowering.cpp      (revision 323454)
+++ lib/Target/PowerPC/PPCISelLowering.cpp      (working copy)
@@ -7128,7 +7128,9 @@
                                  SINT, DAG.getConstant(53, dl, MVT::i32));
       Cond = DAG.getNode(ISD::ADD, dl, MVT::i64,
                          Cond, DAG.getConstant(1, dl, MVT::i64));
-      Cond = DAG.getSetCC(dl, MVT::i32,
+      EVT SVT = getSetCCResultType(DAG.getDataLayout(), *DAG.getContext(),
+                                   MVT::i32);
+      Cond = DAG.getSetCC(dl, SVT,
                           Cond, DAG.getConstant(1, dl, MVT::i64), ISD::SETUGT);

       SINT = DAG.getNode(ISD::SELECT, dl, MVT::i64, Cond, Round, SINT);