llvm dev - Jul 2014 - [LLVMdev] LLVM commit 410f38e01597120b41e406ec1cea69127463f9e5

2014-07-08 12:11 GMT-07:00 Matt Arsenault <Matthew.Arsenault at amd.com>:
>  On 07/07/2014 09:47 PM, deadal nix wrote:
>
>  OK from what I understand, the DAG.getSExtOrTrunc(SetCC, DL, SelectVT)
> is unecessary and the SelectVT is nto the right type (as it is called with
> incorrect parameter).
>
>  Here is a patch so it won't generate a loop. I ran make check and it
> doesn't look like anything is broken.
>
> No, it is necessary and is the fundamental change in that commit. SelectVT
> is not necessarily the same as VT, so it needs to be converted. This patch
> breaks the testcases I have that this fixed. If it helps, this is the
> testcase I was fixing.
>
> define i32 @test_sext_icmp_i64(i64 %arg) {
>   %x = icmp eq i64 %arg, 0
>   %y = sext i1 %x to i32
>   ret i32 %y
> }
>
>
So this is not possible ?
(select (i64 setcc ...), (i32 -1), (i32 0))

>  The setcc type is i64 for the i64 icmp, so it then needs to be truncated
> to 32-bits in this situation
>
>
I understand this? Why does the setcc needs to be truncated ? It doesn't
look like it usually is.

I have a test case which is exactly the other way around :
define i32 @test_sext_icmp_i32(i32 %arg) {
  %x = icmp eq i32 %arg, 0
  %y = sext i1 %x to i64
  ret i64 %y
}

The DAG.getSExtOrTrunc(SetCC, DL, SelectVT) is returning the node
DAGCombiner is working on, which create a loop after the substitution. If I
have X = (i64 (sext (i32 (setcc A B)))) to be "combined" as (select X,
-1,
0) and then, after substitution occurs, the select become its own
condition, creating a loop in the DAG.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140708/36d94515/attachment.html>

On 07/08/2014 02:03 PM, deadal nix wrote:
>
>
>
> 2014-07-08 12:11 GMT-07:00 Matt Arsenault <Matthew.Arsenault at amd.com 
> <mailto:Matthew.Arsenault at amd.com>>:
>
>     On 07/07/2014 09:47 PM, deadal nix wrote:
>>     OK from what I understand, the DAG.getSExtOrTrunc(SetCC, DL,
>>     SelectVT) is unecessary and the SelectVT is nto the right type
>>     (as it is called with incorrect parameter).
>>
>>     Here is a patch so it won't generate a loop. I ran make check
and
>>     it doesn't look like anything is broken.
>     No, it is necessary and is the fundamental change in that commit.
>     SelectVT is not necessarily the same as VT, so it needs to be
>     converted. This patch breaks the testcases I have that this fixed.
>     If it helps, this is the testcase I was fixing.
>
>     define i32 @test_sext_icmp_i64(i64 %arg) {
>       %x = icmp eq i64 %arg, 0
>       %y = sext i1 %x to i32
>       ret i32 %y
>     }
>
>
> So this is not possible ?
> (select (i64 setcc ...), (i32 -1), (i32 0))
>
>     The setcc type is i64 for the i64 icmp, so it then needs to be
>     truncated to 32-bits in this situation
>
>
> I understand this? Why does the setcc needs to be truncated ? It 
> doesn't look like it usually is.
>
> I have a test case which is exactly the other way around :
> define i32 @test_sext_icmp_i32(i32 %arg) {
>   %x = icmp eq i32 %arg, 0
>   %y = sext i1 %x to i64
>   ret i64 %y
> }
>
> The DAG.getSExtOrTrunc(SetCC, DL, SelectVT) is returning the node 
> DAGCombiner is working on, which create a loop after the substitution. 
> If I have X = (i64 (sext (i32 (setcc A B)))) to be "combined" as 
> (select X, -1, 0) and then, after substitution occurs, the select 
> become its own condition, creating a loop in the DAG.
>
I think this will work if we pull the conversion from out of the select, 
so instead select (getSExtOrTrunc) we do getSExtOrTrunc (select). 
However, my target would much rather do the 32-bit select than the 
64-bit select (although arguably i32 trunc (i64 select) -> select (i32 
trunc i64) should be a separate combine. Alternatively maybe this should 
only be done if the setcc type is the same as the sext result?

Does this patch work for you?
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140708/bb38867b/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Move-sext-trunc-out-of-select.patch
Type: text/x-patch
Size: 1330 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140708/bb38867b/attachment.bin>

On 07/08/2014 03:20 PM, Matt Arsenault wrote:
> Alternatively maybe this should only be done if the setcc type is the 
> same as the sext result?
I think we should actually do this. If you need to convert the setcc 
result after, you aren't really gaining anything by doing this 
transformation
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140708/37084110/attachment.html>