[PATCH] D125212: [GlobalISel] Implement the Has{No}Use builtin predicates

Jay Foad via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 9 02:38:49 PDT 2022


foad added inline comments.


================
Comment at: llvm/test/TableGen/GlobalISelEmitter-HasUse.td:45
+
+let HasUse = true in
+defm atomic_load_add_ret : binary_atomic_op<atomic_load_add>;
----------------
A philosophical question: do we really need HasUse? After all the atomic_load_add_ret pattern will still work in both cases (use & nouse), so shouldn't we use priorities instead to prefer the atomic_load_add_no_ret pattern when both are applicable?


================
Comment at: llvm/utils/TableGen/GlobalISelEmitter.cpp:5289
   unsigned OpIdx = 0;
-  for (const TypeSetByHwMode &VTy : Src->getExtTypes()) {
+  unsigned N = DstNumDefs < SrcNumDefs ? DstNumDefs : SrcNumDefs;
+  for (unsigned I = 0; I < N; ++I) {
----------------
Use min() ?


================
Comment at: llvm/utils/TableGen/GlobalISelEmitter.cpp:5291-5292
+  for (unsigned I = 0; I < N; ++I) {
+    const TypeSetByHwMode &VTy = Src->getExtType(I);
     (void)VTy;
 
----------------
This looks odd. Does the call to getExtType have side effects?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125212



More information about the llvm-commits mailing list