[PATCH] D67596: AMDGPU/GlobalISel: Allow selection of scalar min/max

Wed Sep 18 06:55:42 PDT 2019

alex-t added inline comments.

================
Comment at: lib/Target/AMDGPU/SOPInstructions.td:422
 def S_MIN_I32 : SOP2_32 <"s_min_i32",
-  [(set i32:$sdst, (UniformBinFrag<smin> i32:$src0, i32:$src1))]
 >;
----------------
arsenm wrote:
> alex-t wrote:
> > As you remove the "!isDiverent" predicate, I'd like to see the test that ensures that V_MIN/MAX etc are not selected for uniform smin/smax/umin/umax IR operations. The test should check not GlobalIsel but current instruction selection case.
> Those should already exist 
Yes. We have min.ll, max.ll and sminmax.ll
All except min.ll have "-amdgpu-scalarize-global-loads=false" flag in RUN line.
That means that they will select v_min/max always.
This was temporary solution a while ago to avoid massive change in the test base.
 We need to remove that flags at least from max.ll and sminmax.ll and update the test until proceeding with current change.

Formally, guarding both VALU and SALU variants with same predicate looks extensive.
I have made small test and tried your change - it works as expected.

Nevertheless, I'd like to have clean test base until we remove odd predicates.
So, my idea is to create the change that removes "-amdgpu-scalarize-global-loads=false"  from min/max tests. After submitting it you can safely go ahead.
Obviously I will try to submit the test change asap.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67596/new/

https://reviews.llvm.org/D67596