[PATCH] D73483: [AMDGPU] fixed divergence driven shift operations selection

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/VOP2Instructions.td:548
 } // End SubtargetPredicate = isGFX6GFX7GFX10
+let SubtargetPredicate = isGFX6GFX7 in {
+defm V_LSHR_B32 : VOP2Inst <"v_lshr_b32", VOP_PAT_GEN<VOP_I32_I32_I32>, srl>;
----------------
alex-t wrote:
> rampitec wrote:
> > Any tests for GFX10 on this?
> Will be an assertion "Opcode does not exist on target"
But we need a gfx10 test to check proper selection of IR shifts. I do not see these tests.


================
Comment at: llvm/test/CodeGen/AMDGPU/GlobalISel/extractelement.ll:982
 ; GPRIDX-NEXT:    s_mov_b32 s1, s3
-; GPRIDX-NEXT:    s_add_u32 m0, s18, -1
+; GPRIDX-NEXT:    s_add_i32 m0, s18, -1
 ; GPRIDX-NEXT:    s_mov_b32 s2, s4
----------------
It does not belong to this change. Please generate a proper diff.


================
Comment at: llvm/test/CodeGen/AMDGPU/GlobalISel/inst-select-add.mir:2
 # NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
-# RUN: llc -march=amdgcn -mcpu=tahiti -run-pass=instruction-select -verify-machineinstrs -o - %s  | FileCheck -check-prefix=GFX6 %s
 # RUN: llc -march=amdgcn -mcpu=fiji -run-pass=instruction-select -verify-machineinstrs -o - %s  | FileCheck -check-prefix=GFX6 %s
 # RUN: llc -march=amdgcn -mcpu=gfx900 -run-pass=instruction-select -verify-machineinstrs -o - %s  | FileCheck -check-prefix=GFX9 %s
----------------
rampitec wrote:
> Ugh... Fiji is GFX8. Instead of dropping test for tahiti just fix the check-prefix for fiji.
Please return the dropped tahiti run line.


================
Comment at: llvm/test/CodeGen/AMDGPU/shift-select.ll:2
+; RUN: llc -march=amdgcn -mcpu=tahiti -stop-after=instruction-select -verify-machineinstrs < %s | FileCheck -enable-var-scope -check-prefixes=GCN,GFX6 %s
+; RUN: llc -march=amdgcn -mcpu=fiji -stop-after=instruction-select -verify-machineinstrs < %s | FileCheck -enable-var-scope -check-prefixes=GCN,GFX8 %s
+
----------------
Please add another line for gfx1010.


================
Comment at: llvm/test/CodeGen/AMDGPU/shift-select.ll:57
+; GFX8: V_ASHRREV_I32_e64
+define amdgpu_kernel void @v_ashr_i32(i32 addrspace(1)* %out, i32 addrspace(1)* %in) {
+  %tid = call i32 @llvm.amdgcn.workitem.id.x()
----------------
You also need tests for i64.


================
Comment at: llvm/test/CodeGen/AMDGPU/shift-select.ll:69
+declare i32 @llvm.amdgcn.workitem.id.x()
\ No newline at end of file

----------------
Newline.


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

https://reviews.llvm.org/D73483