[PATCH] D105042: [SLP][COST][X86]Improve cost model for masked gather.

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: llvm/lib/Target/X86/X86TargetTransformInfo.cpp:4780
+    if (ST->hasAVX512() && (NumElts == 2 || (NumElts == 4 && !ST->hasVLX())))
+      return false;
   }
----------------
RKSimon wrote:
> I'm really not sure about this - by moving this into isLegalMaskedGather we're affecting codegen as well as IR.
> 
> While we agreed that we'd avoid this on AVX2 targets (where legal gathers are actually REALLY bad), more minor cost differences on AVX512 targets its not so clear.
I think this change is ok. Actually, we saw perf degradations with masked gathers even for AVX512 and had to change to use only for at least 8 elements, even for 4 elements simple gathering might be more profitable than the masked gathering. I would agree, that this requires some extra fine-tuning but I believe this kind of check should be in legality rather than in the cost. Without this change, we return the cost of loads+gathering, which is not the cost of the MaskedGather op. And I think this is good for codegen too since it relies on the legality check when chooses between masked gathering and simple gathering.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105042