[PATCH] D50575: [AMDGPU] Add support for a16 modifier for gfx9

[Nicolai Hähnle via Phabricator via llvm-commits]

nhaehnle added a comment.
Herald added a subscriber: jvesely.

The operand name / encoding bits in TableGen should really be changed as well to indicate the overload between R128 and A16.



================
Comment at: lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:294
   bool isDA() const { return isImmTy(ImmTyDA); }
-  bool isR128() const { return isImmTy(ImmTyR128); }
+  bool isR128() const { return isImmTy(ImmTyR128A16); }
   bool isLWE() const { return isImmTy(ImmTyLWE); }
----------------
Name of the function should be adjusted as well.


================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:4625-4626
+  MVT VAddrVT = Op.getOperand(DimIdx).getSimpleValueType();
+  if (VAddrVT.getScalarType() == MVT::f16 && 
+        ST->hasFeature(AMDGPU::FeatureR128A16)) {
+    IsA16 = true;
----------------
Please check formatting with clang-format-diff, this looks wrong.


================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:4633-4634
+        AddrLo = Op.getOperand(i);
+      }
+      else {
+        AddrLo = Op.getOperand(i);
----------------
Here as well.


================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:4637
+        // Dz/dh, dz/dt and the last odd coord are packed with undef. Also,
+        // in 1D, derivatives dx/dh and dy/dh are packed with undef.
+        unsigned DimZIdx = i - DimIdx + 1;
----------------
Should be dz/dv instead of dz/dt, and dx/dv instead of dy/dh.


================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:4639-4641
+        if (((i + 1) >= (AddrIdx + NumMIVAddrs)) || 
+            (NumGradients == 2 && (i < DimIdx + NumGradients)) ||
+            (NumGradients == 6 && (DimZIdx == 3 || DimZIdx == 6))) {
----------------
I think the second part of the condition would be easier to follow as `(NumGradients / 2) % 2 == 1 && (i == DimIdx + (NumGradients / 2) - 1 || i == DimIdx + NumGradients - 1)` ("the number of texture coordinates relevant for the gradients is odd, and the current index is the last gradient with respect to dh or with respect to dv").


Repository:
  rL LLVM

https://reviews.llvm.org/D50575