[PATCH] D142021: AMDGPU/GlobalISel: Add stub custom regbankselect pass

Diana Picus via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 24 06:12:43 PST 2023


rovka accepted this revision as: rovka.
rovka added a comment.

Thanks for the updates, LGTM with nits.



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPURegBankSelect.cpp:11
+// register bank selection. The use/def analyses the default RegBankSelect are a
+// secondary consideration.
+//
----------------
Nit: I like the first sentence, but the second one is a bit difficult to parse. Maybe you can rephrase it a bit?


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPURegBankSelect.cpp:14
+
+#define DEBUG_TYPE "regbankselect"
+
----------------
arsenm wrote:
> rovka wrote:
> > Nit: This diverges from AMDGPUInstructionSelector and Legalizer, which both define the DEBUG_TYPE as "amdgpu-blah" directly.
> Those should probably use the generic name. Different debug types for the same pass are counterproductive
Be that as it may, I'd prefer using the same string for -stop-before/after and -debug-only. As it is now we'd have to use "amdgpu-regbankselect" to run the pass and just "regbankselect" to get the debug logs.


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

https://reviews.llvm.org/D142021



More information about the llvm-commits mailing list