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

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>

The proposed patch would work for me. The combine make sense for me 100% of
the time as my target can have different type for the setcc and the
selected values. So if it is not in DAGCombiner, I'm going to end up doing
it one way or another in the backend.


2014-07-08 16:26 GMT-07:00 Matt Arsenault <Matthew.Arsenault at amd.com>:
>  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/ec128dc0/attachment.html>

Resurrecting this thread.

After quite a lot of discussion with Matt on IRC, here are some conclusion
and some experiment I came up with.

 - getSetCCResultType use should probably limited to find the setcc result
type based on setcc argument type. Using it to find out the expect type of
a select's predicate based on the selected value type is an incorrect use.
The use case here is isolated enough so I'm not sure it really make sense
is backend agnostic code.
 - Removing the optimization altogether break a bunch of tests. Backends
expect it to work.
 - Not truncating or extending the SetCC value seems to work. select node
with different type for predicate and value is already supported, and we
take advantage of this. Note: this used to break some tests on x86, but it
doesn't anymore, so I suppose x86 target is now capable of handling this
properly.
 - I tried to Truncate if SetCC is larger than the selected values but keep
different types if it is extended. This also seems to work properly.
 - Using another type of extend (zero extend for instance) produce idiotic
results.
 - doing the sext or trunc after the select (sext_or_trunc (select, setcc,
0, -1)) work as well but will make x86 generate 64 bit select when 32 bits
one would be sufficient. This either need to be handled in the target (but
in this case, going for a more general solution is preferable IMO) so this
seems to be the wrong balance.

Basically, the deeper question here is do we want to support predicate of
different type that selected value. I think we want considering the various
option explored here.


2014-07-08 22:42 GMT-07:00 deadal nix <deadalnix at gmail.com>:
> The proposed patch would work for me. The combine make sense for me 100%
> of the time as my target can have different type for the setcc and the
> selected values. So if it is not in DAGCombiner, I'm going to end up
doing
> it one way or another in the backend.
>
>
> 2014-07-08 16:26 GMT-07:00 Matt Arsenault <Matthew.Arsenault at
amd.com>:
>
>>  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/20140921/be6aea4e/attachment.html>