Revert "[AMDGPU] Enable code selection using `s_mul_hi_u32`/`s_mul_hi_i32`."

[kzhuravl]

Summary

It introduces performance regressions in several applications.

[rampitec]

lib/Target/AMDGPU/SIInstrInfo.cpp

3341 ↗

(On Diff #198063)

I think you may inhibit the pattern, but there must be a way to move it to VALU.

[arsenm]

Should not revert to fix performance problems. A proper fix should be found

[arsenm]

Reverting this would also hide / complicate another issue I’m working on

[rampitec]

lib/Target/AMDGPU/SIInstrInfo.cpp

3341 ↗

(On Diff #198063)

I mean just retain getVALUOp() changes, you only need to revert patterns.

[kzhuravl]

Address review feedback.

[kzhuravl]

In D61528#1490289, @arsenm wrote:

Should not revert to fix performance problems. A proper fix should be found

There are several time sensitive applications which are affected. If no objections, I will file a ticket to re-instate the patterns once I get time to investigate why they are causing perf regresssions.

[rampitec]

LGTM

[rampitec]

lib/Target/AMDGPU/SOPInstructions.td

561

Actually it is still isCommutable, even if you drop the pattern.

[rampitec]

lib/Target/AMDGPU/SOPInstructions.td

561

But I think it's OK to restore it later while working on the regression.

[kzhuravl]

rL361879

lib/Target/AMDGPU/SOPInstructions.td

let SubtargetPredicate = isGFX9Plus in {
 
   let Defs = [SCC] in {
     def S_LSHL1_ADD_U32 : SOP2_32<"s_lshl1_add_u32">;
     def S_LSHL2_ADD_U32 : SOP2_32<"s_lshl2_add_u32">;
     def S_LSHL3_ADD_U32 : SOP2_32<"s_lshl3_add_u32">;
     def S_LSHL4_ADD_U32 : SOP2_32<"s_lshl4_add_u32">;
   } // End Defs = [SCC]
 
-  let isCommutable = 1 in {
-    def S_MUL_HI_U32 : SOP2_32<"s_mul_hi_u32",
-      [(set i32:$sdst, (UniformBinFrag<mulhu> SSrc_b32:$src0, SSrc_b32:$src1))]>;
-    def S_MUL_HI_I32 : SOP2_32<"s_mul_hi_i32",
-      [(set i32:$sdst, (UniformBinFrag<mulhs> SSrc_b32:$src0, SSrc_b32:$src1))]>;
-  }
+  def S_MUL_HI_U32 : SOP2_32<"s_mul_hi_u32">;
+  def S_MUL_HI_I32 : SOP2_32<"s_mul_hi_i32">;
 } // End SubtargetPredicate = isGFX9Plus
 
 //===----------------------------------------------------------------------===//
 // SOPK Instructions
 //===----------------------------------------------------------------------===//
 
 class SOPK_Pseudo <string opName, dag outs, dag ins,
                    string asmOps, list<dag> pattern=[]> :

test/CodeGen/AMDGPU/mul.ll

 ; A standard 64-bit multiply.  The expansion should be around 6 instructions.
 ; It would be difficult to match the expansion correctly without writing
 ; a really complicated list of FileCheck expressions.  I don't want
 ; to confuse people who may 'break' this test with a correct optimization,
 ; so this test just uses FUNC-LABEL to make sure the compiler does not
 ; crash with a 'failed to select' error.
 
 ; FUNC-LABEL: {{^}}s_mul_i64:
-; GFX9_10-DAG: s_mul_i32
-; GFX9_10-DAG: s_mul_hi_u32
-; GFX9_10-DAG: s_mul_i32
-; GFX9_10-DAG: s_mul_i32
-; GFX9_10: s_endpgm
 define amdgpu_kernel void @s_mul_i64(i64 addrspace(1)* %out, i64 %a, i64 %b) nounwind {
   %mul = mul i64 %a, %b
   store i64 %mul, i64 addrspace(1)* %out, align 8
   ret void
 }
 
 ; FUNC-LABEL: {{^}}v_mul_i64:
 ; GCN: v_mul_lo_u32