[PATCH] D103770: Injection of kernel features into the LLVM IR during the OpenMP transform stage

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

Some minor thing but overall this looks sensible.



================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:571
+
+      Changed |= injectKernelFeatures();
     }
----------------
We need a command line option to guard this. Doing it always is (for now) not helpful.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:1507-1508
+        IsChanged = true;
+        LLVM_DEBUG(FeatureArray->print(dbgs() << TAG << " Function info for kernel " << F->getName() << " is: "));
+        LLVM_DEBUG(dbgs() << "\n");
+      }
----------------
Or use LLVM_DEBUG({ ... }); to avoid two consecutive ones.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:1513
+        LLVM_DEBUG(dbgs() << TAG << " No valid function info received for kernel " << F->getName() << "\n");
+      }
+    }
----------------
Early exits are preferred. `if (!FunctionInfo) { debug; continue; }` will make it easier to read.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103770