> Date: Tue, 28 Apr 2020 01:31:12 -0400
> From: George Koehler <kern...@gmail.com>
> 
> 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 <juliana...@posteo.jp> 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?

Upstream fixed this issue as well.  Apparently only ADDE can't be
legalized (because it is "special") but ADDCARRY can.  Do ypu want to
adjust your diff based on that information?

Either way, ok kettenis@


> 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