[llvm] [AMDGPU][True16] Support V_CEIL_F16. (PR #73108)

Joe Nash via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 1 09:16:30 PST 2023


================
@@ -2303,7 +2303,17 @@ bool AMDGPUOperand::isInlineValue() const {
 // AsmParser
 //===----------------------------------------------------------------------===//
 
-static int getRegClass(RegisterKind Is, unsigned RegWidth) {
+static int getRegClass(RegisterKind Is, unsigned RegWidth,
+                       RegisterSuffix Suffix) {
+  if (Suffix != RegisterSuffix::None) {
+    if (Is == IS_VGPR && RegWidth == 16) {
+      if (Suffix == RegisterSuffix::Lo)
+        return AMDGPU::VGPR_LO16RegClassID;
----------------
Sisyph wrote:

This proposed design uses 2 new register classes (or is it 4 if you need to have VGPR_HI16_Lo128 and VGPR_Lo16_Lo128 as well) whereas the other needs 2 lines to compute RegIdx. I don't think that's warranted.

If you still think this way is better, please prepare a patch downstream to show the benefit. Or alternatively, you could try removing VGPR_HI16 and VGPR_LO16 right away.

It is extremely difficult to review changes that contain 1) porting of downstream code, 2) transition code necessary for the true16 and fake16 versions to coexist and 3) refactoring/alternative implementations. Please separate them as much as possible. Refactoring could be done either downstream in advance of upstreaming, or on upstream code after direct porting.



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


More information about the llvm-commits mailing list