[PATCH/RFC] fold zext(setcc) into a larger setcc

[Fiona Glaser]

> On Jan 27, 2015, at 12:00 AM, Owen Anderson <resistor at mac.com> wrote:
> 
>> 
>> On Jan 26, 2015, at 5:39 PM, Fiona Glaser <fglaser at apple.com> wrote:
>> 
>>> The aext -> select_cc here is OK, but in your patch it will combine zext (setcc) -> setcc. On a target that uses ZeroOrNegativeOneBooleanContent, that would produce a wider type with 0 upper bits which is not the same as the all 1 bits the larger compare result would give. There are also a lot of pre-existing bugs that mishandle this now that still assume 0/1 setcc results.
>>> 
>>> I guess I don't understand why this wouldn't already be happening during legalization if getSetCCResultType gives i32. I would not expect two different result types for the same setcc to ever work.
>>> 
>>> -Matt
>>> 
>> 
>> What should one do if there are multiple valid SetCC result types?
>> 
>> I checked the code for the out-of-tree target, and it has i16 set to the default result type, but the target also supports i32. Is the problem that LLVM doesn’t support optimizing for the case of a target that can pick and choose its SetCC result type?
> 
> It seems like the best solution might be to optimize the sequence to SELECT_CC instead of SETCC if the type in question is not the default SETCC type for the target.  I believe you can find *that* out by checking getSetCCResultType.
> 
> —Owen

I just tried this and ran into some very weird results.

So the current code does this:

    // zext(setcc x,y,cc) -> select_cc x, y, 1, 0, cc
    SDValue SCC =
      SimplifySelectCC(SDLoc(N), N0.getOperand(0), N0.getOperand(1),
                       DAG.getConstant(1, VT), DAG.getConstant(0, VT),
                       cast<CondCodeSDNode>(N0.getOperand(2))->get(), true);
    if (SCC.getNode()) return SCC;

which feels a bit broken, because the function called only succeeds if it manages to *simplify* the SELECT_CC, as far as I see, and it fails in this case because there’s no simplification to be done. So I added this after:


    if (!LegalOperations || TLI.isOperationLegal(ISD::SELECT_CC, VT))
      return DAG.getNode(ISD::SELECT_CC, SDLoc(N), VT, N0.getOperand(0),
                         N0.getOperand(1), DAG.getConstant(1, VT),
                         DAG.getConstant(0, VT), N0.getOperand(2));

and this actually causes the DAG to infinite-loop, because it then goes and breaks up the SELECT_CC into a SETCC + ZEXT later, presumably, then combines it again. This *also* affects the out-of-tree target, because despite the setcc/select_cc both being legal operations with VT==i32, the DAG nevertheless tries to canonicalize it into an infinite loop.

Fiona
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150127/57c5b5c2/attachment.html>