[PATCH] D155867: [AMDGPU][GlobalISel] Fix applyMappingImpl function for G_ABS and type v2s16

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

lgtm with some nits. Can you fix the patch title to something that isn't "fix"? I don't think anything was wrong here, just sub-optimal



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp:2427-2435
+        Register WideSrcLo, WideSrcHi;
+
+        std::tie(WideSrcLo, WideSrcHi)
+          = unpackV2S16ToS32(B, MI.getOperand(1).getReg(), TargetOpcode::G_SEXT);
+        auto Lo = B.buildInstr(AMDGPU::G_ABS, {S32}, {WideSrcLo});
+        auto Hi = B.buildInstr(AMDGPU::G_ABS, {S32}, {WideSrcHi});
+        B.buildBuildVectorTrunc(DstReg, {Lo.getReg(0), Hi.getReg(0)});
----------------
indentation looks off


================
Comment at: llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.abs.vector.ll:5-8
+declare <2 x i8> @llvm.abs.v2i8(<2 x i8>, i1)
+declare <3 x i8> @llvm.abs.v3i8(<3 x i8>, i1)
+declare <2 x i16> @llvm.abs.v2i16(<2 x i16>, i1)
+declare <3 x i16> @llvm.abs.v3i16(<3 x i16>, i1)
----------------
Should just add these cases to the existing llvm.abs.ll tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155867