<div dir="ltr">Which target ? I can try to figure out a solution. As per submitted test cases, this do not affect X86 or X86-64 int that very scenario, the 32 bit select is generated properly.<br></div><div class="gmail_extra"><br><div class="gmail_quote">2014-09-25 13:57 GMT-07:00 Matt Arsenault <span dir="ltr"><<a href="mailto:arsenm2@gmail.com" target="_blank">arsenm2@gmail.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><br>
On Sep 25, 2014, at 1:42 PM, Hal Finkel <<a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>> wrote:<br>
<br>
> ----- Original Message -----<br>
>> From: "deadal nix" <<a href="mailto:deadalnix@gmail.com">deadalnix@gmail.com</a>><br>
>> To: "llvm-commits" <<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a>><br>
>> Sent: Thursday, September 25, 2014 12:09:07 AM<br>
>> Subject: Remove possible loop creation in DAGCombiner<br>
>><br>
>><br>
>><br>
>><br>
>> DAGCombiner can create loop in the DAG when you sext the result of a<br>
>> setcc, depending on the target.<br>
>><br>
>> This diff remove the call to DAG.getSExtOrTrunc that create the loop.<br>
>> This call used to be necessary for x86 backend, but it doesn't look<br>
>> necessary anymore. I've added tests to ensure it doesn't.<br>
><br>
> -        EVT SelectVT = getSetCCResultType(VT);<br>
> -        return DAG.getSelect(DL, VT,<br>
> -                             DAG.getSExtOrTrunc(SetCC, DL, SelectVT),<br>
> -                             NegOne, DAG.getConstant(0, VT));<br>
> -<br>
> +        return DAG.getSelect(DL, VT, SetCC,<br>
> +                          NegOne, DAG.getConstant(0, VT));<br>
>       }<br>
><br>
> It seems clear to me that creating an SIGN_EXTEND inside of visitSIGN_EXTEND could easily lead to a loop, and we should avoid that. However, getSExtOrTrunc might also have been providing a TRUNC, and if the SIGN_EXTEND really would be needed, we may just want to abort the transformation all together. For one thing, I don't think that it can be different unless VT != N0.getOperand(0).getValueType().<br>
<br>
</span>I do need the trunc behavior, for the case of selecting between 32-bit values based on a 64-bit comparison on a target which has the width of the setcc type change depending on the width of the compared operands<br>
<span class=""><br>
><br>
> In short, you're proposing to change target-independent code, and I'd be more conservative. Why don't you just disable the transformation when SelectVT.bitsGT(SetCC.getValueType()) is true (which I believe is the condition under which we'd get the SIGN_EXTEND).<br>
<br>
</span>I agree, and what I was planning on trying when I got back to fixing that patch.<br>
<div class="HOEnZb"><div class="h5"><br>
><br>
> -Hal<br>
><br>
>><br>
>><br>
>><br>
>> _______________________________________________<br>
>> llvm-commits mailing list<br>
>> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
>><br>
><br>
> --<br>
> Hal Finkel<br>
> Assistant Computational Scientist<br>
> Leadership Computing Facility<br>
> Argonne National Laboratory<br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br>
</div></div></blockquote></div><br></div>