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

Matt Arsenault via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 14 17:18:46 PDT 2015


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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151014/408c2f93/attachment.html>


More information about the llvm-commits mailing list