[PATCH] D12007: [AMDGPU] Use the general SMAX/SMIN/UMAX/UMIN pattern matching and remove the AMDGPU implementation

James Molloy via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 15 00:58:11 PDT 2015


Hi Matt,

Would it not be sufficient to have a simple custom expand for these nodes?

In fact, thinking about it, isn’t the fault in the legalizer? If we’re expanding a vector type, and we know the scalar version is legal, we should expand to that instead of to the select pattern. This seems like an easy fix.

James
> On 15 Oct 2015, at 01:18, Matt Arsenault <Matthew.Arsenault at amd.com> wrote:
>
> On 08/13/2015 01:43 PM, James Molloy wrote:
>> Hi Matt,
>>
>> Yes; min/max is now matched at the IR level. I know there is the possibility of this approach missing some min/max's exposed at the DAGCombine level, but that could be fixed with point fixes if a common pattern emerges.
>>
>> The advantage of matching at the SDAGBuilder stage is that all the (fairly complex) logic gets kept in one place. We've now (or will have by the end of the week) removed 4 separate implementations of matching mins/maxs, all dealing with the subtleties of orderedness/unorderedness/signed-zeroes/NaNs. I hope that in the long run this new approach will be more maintainable, and I'm sure we can patch up any regressions in DAGCombine.
>>
>> James
>
> I've found a problem with this approach. On AMDGPU we don't have any vector operations, but legal vector types. We only really care about legality of the scalar operations, but because we have the min/max nodes set to expand on vectors, the IR pattern matching doesn't trigger. I think we should bring back a DAG pattern match for this to catch these cases
>
> -Matt


________________________________

-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


More information about the llvm-commits mailing list