<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">2014-09-28 0:12 GMT-07:00 Hal Finkel <span dir="ltr"><<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>></span>:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="">----- Original Message -----<br>
> From: "deadal nix" <<a href="mailto:deadalnix@gmail.com">deadalnix@gmail.com</a>><br>
</span><span class="">> To: "Hal Finkel" <<a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>><br>
> Cc: "llvm-commits" <<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a>>, "Nadav Rotem" <<a href="mailto:nrotem@apple.com">nrotem@apple.com</a>>, "Owen Anderson" <<a href="mailto:resistor@mac.com">resistor@mac.com</a>><br>
> Sent: Friday, September 26, 2014 3:46:32 AM<br>
> Subject: Re: Remove possible loop creation in DAGCombiner<br>
><br>
> I tried various options:<br>
><br>
> - Not do the transformation when size do not match. This break<br>
> several backends as the sext is done from a 1bit value which most<br>
> backend do not expect or support. This needs to be transformed into<br>
> a select one way or another.<br>
<br>
</span>This seems odd, type legalization should take care of that problem (and DAGCombine should not introduce illegal types after type legalization is complete). What transformation in producing these illegal nodes?<br>
<span class=""><br></span></blockquote><div><br></div><div>The problem is that the sext from a setcc node is unexpected by several in tree backends. I didn't invetigated in details, simply noticed that there was a lot of broken ciodegen tests .<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="">
> - do the select with the setcc size, and then resize the select. It<br>
> does work but produce inefficient code in various situations (select<br>
> a 64 bit and resize the result to 32bits).<br>
><br>
> - trunc but never extend. This yield the same results as the proposed<br>
> diff.<br>
<br>
</span>What does "same results" mean?<br>
<span class=""><br></span></blockquote><div><br></div><div>That the codegen were the same for all tests I looked at.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="">
><br>
> - the proposed diff.<br>
><br>
> Ultimately, the question is do the backend must handle slect with<br>
> different size for selected values and predicate. It look like they<br>
> do (and other part of the toolchain generate such code).<br>
<br>
</span>As documented, the sizes of the selected values and the predicate may not match. include/llvm/CodeGen/ISDOpcodes.h says:<br>
<br>
/// Select(COND, TRUEVAL, FALSEVAL). If the type of the boolean COND is not<br>
/// i1 then the high bits must conform to getBooleanContents.<br>
SELECT,<br>
<br>
and so the SIGN_EXTEND there is generically wrong anyway (it is correct only for ZeroOrNegativeOneBooleanContent).<br>
<span class=""><br></span></blockquote><div><br></div><div>Then I guess we should simply not do it.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="">
><br>
> Another thing worth noticing is the use of getSetCCResultType with<br>
> the selected value's type as parameter. This is a different semantic<br>
> than other uses of the method (predicate's argument type is passed).<br>
> It seems that this is an incorrect use of that function to begin<br>
> with, as there is no reason that the 2 respond to the same<br>
> constraint, and, in the case of the select, this is not given that<br>
> there is a constraint at all. Basing any decision on the result of<br>
> an incorrect use of that function will be a source of trouble.<br>
<br>
</span>This is unfortunately somewhat confusing. getSetCCResultType is documented as:<br>
<br>
/// Return the ValueType of the result of SETCC operations. Also used to<br>
/// obtain the target's preferred type for the condition operand of SELECT and<br>
/// BRCOND nodes. In the case of BRCOND the argument passed is MVT::Other<br>
/// since there are no other operands to get a type hint from.<br>
virtual EVT getSetCCResultType(LLVMContext &Context, EVT VT) const;<br>
<br>
so it seems to be a matter of determining what "preference" means here, and whether backends deal reasonably with non-preferred choices.<br>
<br>
Thanks again,<br>
Hal<br>
<div class=""><div class="h5"><br></div></div></blockquote><div><br></div><div>That do not make a lot of sense to me. There is no reason for a target to have the same preference in both scenarios. In fact the one I'm working on will generate a result that has the same size as argument for the setcc, and do not care about the size of the predicate. If I select a 64bits values from a 32bits predicate, I get a loop in the dag:<br><br></div><div>(sext i64 (setcc i32:X i32:Y)) =>(select (sext i64 (setcc i32:X i32:Y)) -1 0)<br><br></div><div>When you proceed to the substitution, you end up with a loop:<br></div><div>N = (select N -1 0)<br><br></div><div>To make the right decision in the case of the select, the backend need to have information about the type of the predicate and the type of the selected values. One of them is insufficient.<br></div></div></div></div>