[PATCH] D80749: AMDGPU/GlobalISel: cmp/select method for extract element

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec marked 6 inline comments as done.
rampitec added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUPostLegalizerCombiner.cpp:230-231
+
+  // FIXME: We need another combiner post RegBankSelect. Then move this combine
+  //        there and actually query RegBank of the Idx.
+  bool IsDivergent = false;
----------------
arsenm wrote:
> For the 2 element case, we should do this in the pre-legalizer combiner (I guess the 16-bit case code would be different, since it would be bitcast and shift)
Why is that? The problem is we need to know if index is divergent or not in general, so we avoid creating loops. I assume if we need some special case for 2 elements, that shall be a separate combine.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUPostLegalizerCombiner.cpp:258
+    Register Elt = MRI.createGenericVirtualRegister(EltTy);
+    B.buildExtract(Elt, Vec, I * EltSize);
+    if (I == 0) {
----------------
arsenm wrote:
> It would be better to produce a single G_UNMERGE_VALUES and index out of the result list than to produce a sequence of G_EXTRACTs.
> 
> Also FYI I'm planning on making 16-bit vector indexing bitcast promote to 32-bit vectors, so this would only see 32 and 64-bit element vectors
Thanks. Apparently there is a bug in the CombinerHelper::matchEqualDefs() when it works with an opcode producing multiple values. I will have to fix it first.


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

https://reviews.llvm.org/D80749