[PATCH] D144911: adding bf16 support to NVPTX

[Jacques Pienaar via Phabricator via llvm-commits]

jpienaar added a comment.

Thanks Artem for explaining,



================
Comment at: llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp:414-415
   addRegisterClass(MVT::v2f16, &NVPTX::Float16x2RegsRegClass);
-  addRegisterClass(MVT::bf16, &NVPTX::Float16RegsRegClass);
-  addRegisterClass(MVT::v2bf16, &NVPTX::Float16x2RegsRegClass);
+  addRegisterClass(MVT::bf16, &NVPTX::BFloat16RegsRegClass);
+  addRegisterClass(MVT::v2bf16, &NVPTX::BFloat16x2RegsRegClass);
 
----------------
tra wrote:
> Is there a particular reason we need to create another register class for `.b16` and `.b32` registers? 
> I think ideally register classes should represent the actual register types available in PTX. While f16/bf16, etc are operation types, they are not *register* types.
> 
> One of the side effects of adding multiple aliases for the actual register types that we end up with LLVM generating unnecessary moves just to convert between different register classes.
> 
> We've had that pesky behavior with fp16 and I was attempting to avoid creating new redundant register classes for bf16 when I've added storage-only support for bf16. The name `Float16*RegsRegClass` should probably get a better name to reflect that it represents an opaque 16 or 32-bit register, but it did represent those registers just fine.
> 
This is marked as done, but I don't see a new change here - did you upload the latest commit?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144911