Moving from bugs@ to tech@,
because some people might miss a clang diff on bugs@.

This diff modifies LLVM's DAGCombiner to skip an optimization if it
would make an illegal ISD::ADDE node.  This fixes fatal errors from
powerpc clang when building ports net/libtorrent-rasterbar and
devel/avr/binutils on PowerPC.  The fatal errors look like,

On Sun, 19 Apr 2020 17:26:49 +0200
Charlene Wendling <[email protected]> wrote:

> fatal error: error in backend: Cannot select: 0x90a1c1b0: i1,glue = adde 
> Constant:i1<0>, Constant:i1<-1>, 0xa6a2b1b0:1
>   0xa6a2b828: i1 = Constant<0>
>   0x90a1c630: i1 = Constant<-1>
>   0xa6a2b1b0: i32,glue = addc 0x90a1c3a8, Constant:i32<-1>

An example to cause this error, given unsigned 32-bit *p, is
        if (*p > ((*p + 15ULL) & ~15ULL)) ...

The SelectionDAG goes through phases.  The DAG starts with an i64 add
*p + 15ULL, but 32-bit PowerPC has no i64 ops, so the legalize-types
phase converts the i64 add to an i32 addc/adde pair.  The DAG keeps
some i1 ops because clang -mcrbits (the default) enables i1 ops on
condition register bits.  DAGCombiner is optimizing the i1 ops when it
truncates the i32 adde to an i1 adde, but i1 adde is an illegal op.

Legalize-ops runs next, and would convert illegal ops to legal ops,
but it can't fix i1 adde, because it has no case for ISD::ADDE.
LegalizeIntegerTypes.cpp:1966 knows this, and refuses to insert an
illegal adde, but DAGCombiner::visitTRUNCATE didn't know.

This diff might affect other arches.  I guess that Aarch64, ARM, MIPS,
Sparc don't have integer ops smaller than i32, and X86 doesn't use
ISD::ADDE, so this diff might not affect them.

OK to commit?

Index: lib/CodeGen//SelectionDAG/DAGCombiner.cpp
===================================================================
RCS file: /cvs/src/gnu/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp,v
retrieving revision 1.1.1.8
diff -u -p -r1.1.1.8 DAGCombiner.cpp
--- lib/CodeGen//SelectionDAG/DAGCombiner.cpp   23 Jun 2019 21:36:48 -0000      
1.1.1.8
+++ lib/CodeGen//SelectionDAG/DAGCombiner.cpp   27 Apr 2020 21:48:18 -0000
@@ -9904,9 +9904,10 @@ SDValue DAGCombiner::visitTRUNCATE(SDNod
   // (trunc adde(X, Y, Carry)) -> (adde trunc(X), trunc(Y), Carry)
   // (trunc addcarry(X, Y, Carry)) -> (addcarry trunc(X), trunc(Y), Carry)
   // When the adde's carry is not used.
+  // Don't make an illegal adde: LegalizeDAG can't expand nor promote it.
   if ((N0.getOpcode() == ISD::ADDE || N0.getOpcode() == ISD::ADDCARRY) &&
       N0.hasOneUse() && !N0.getNode()->hasAnyUseOfValue(1) &&
-      (!LegalOperations || TLI.isOperationLegal(N0.getOpcode(), VT))) {
+      TLI.isOperationLegal(N0.getOpcode(), VT)) {
     SDLoc SL(N);
     auto X = DAG.getNode(ISD::TRUNCATE, SL, VT, N0.getOperand(0));
     auto Y = DAG.getNode(ISD::TRUNCATE, SL, VT, N0.getOperand(1));

Reply via email to