[PATCH] D144911: adding bf16 support to NVPTX

Tue Jun 13 14:50:33 PDT 2023

tra accepted this revision.
tra added a comment.
This revision is now accepted and ready to land.

LGTM with few nits. Thank you for your patience with revising the patch.

================
Comment at: llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp:629-631
+  const bool IsBFP16FP16x2NegAvailable = STI.getSmVersion() >= 80 &&
+                                         STI.getPTXVersion() >= 70 &&
+                                         STI.hasBF16Math();
----------------
IsBFP16FP16x2NegAvailable is no longer used and can be removed.

================
Comment at: llvm/lib/Target/NVPTX/NVPTXInstrInfo.td:159
 def useFP16Math: Predicate<"Subtarget->allowFP16Math()">;
+def useBF16Math: Predicate<"Subtarget->hasBF16Math()">;

----------------
Nit: I'd rename the record to `hasBF16Math` as the decision is based purely on whether we have particular features enabled, and not on whether user input allows us to use those instructions or not on the hardware where they are present.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144911