Bug in SUB expansion going back to LLVM 2.6

Project Infrastructure LLVM Dev List Archives
1

I found a bug in the expansion code for SUB going back to at least LLVM 2.6 and still shows up in trunk.

case ISD::SUB: {

EVT VT = Node->getValueType(0);

assert(TLI.isOperationLegalOrCustom(ISD::ADD, VT) &&

TLI.isOperationLegalOrCustom(ISD::XOR, VT) &&

“Don’t know how to expand this subtraction!”);

Tmp1 = DAG.getNode(ISD::XOR, dl, VT, Node->getOperand(1),

DAG.getConstant(APInt::getAllOnesValue(VT.getSizeInBits()), VT));

Tmp1 = DAG.getNode(ISD::ADD, dl, VT, Tmp2, DAG.getConstant(1, VT));

Results.push_back(DAG.getNode(ISD::ADD, dl, VT, Node->getOperand(0), Tmp1));

break;

}

The problem is Tmp2 is not initialized and should be Tmp1 instead. This code only is hit if the architecture does not support ‘SUB’ and you need to expand it.

Patch is attached, please commit if good.

isd_sub_expand.diff (672 Bytes)

2

It seems to me that a test case demonstrating bad code would be helpful here.
[ Not that I'm suggesting that the current code is correct… ]

-- Marshall

Marshall Clow Idio Software <mailto:mclow.lists@gmail.com>

A.D. 1517: Martin Luther nails his 95 Theses to the church door and is promptly moderated down to (-1, Flamebait).
        -- Yu Suzuki

3

Here is a test case:

define i32 @sub_expand_test(i32 %a, i32 %b) nounwind {

entry:

%tmp5 = sub i32 %b, %a

ret i32 %tmp5

}

The problem with this is that unless you modified the *ISelLowering.cpp class to set ISD::SUB to ‘Expand’, you won’t trigger this.

4

Applied in r157215.

--Owen