[Mlir-commits] [mlir] [AMDGPU] [MLIR] Add 96 and 128 bit GatherToLDS for gfx950 (PR #147496)

[Daniel Hernandez-Juarez]

================
@@ -1206,11 +1206,11 @@ struct GatherToLDSOpLowering : public ConvertOpToLLVMPattern<GatherToLDSOp> {
     }();
 
     // Currently only 1, 2, 4, 12 and 16 byte loads are supported.
-    if (loadWidth != 1 && loadWidth != 2 && loadWidth != 4 && loadWidth != 12 &&
-        loadWidth != 16)
+    if (!llvm::is_contained(ArrayRef<size_t>{1, 2, 4, 12, 16}, loadWidth))
       return op.emitOpError("chipset unsupported element size");
 
-    if (chipset != kGfx950 && (loadWidth == 12 || loadWidth == 16))
+    if (chipset != kGfx950 &&
+        llvm::is_contained(ArrayRef<size_t>{12, 16}, loadWidth))
----------------
dhernandez0 wrote:

you are right but if we don't do this we get warnings when compiling:
`/home/danherna/mlir-dev/llvm-project/llvm/include/llvm/ADT/STLExtras.h:1932:11: warning: comparison of integers of different signs: 'const int' and 'const unsigned long' [-Wsign-compare]
 1932 |     if (V == Element)
      |         ~ ^  ~~~~~~~
/home/danherna/mlir-dev/llvm-project/mlir/lib/Dialect/AMDGPU/IR/AMDGPUDialect.cpp:514:14: note: in instantiation of function template specialization 'llvm::is_contained<int, unsigned long>' requested here
  514 |   if (!llvm::is_contained({8, 16, 32, 96, 128}, transferSize))`

I don't have a strong opinion about this. What do you prefer?

https://github.com/llvm/llvm-project/pull/147496