[Openmp-commits] [PATCH] D106033: [OpenMP] Folding threadLimit and numThreads when single value in kernels

[Johannes Doerfert via Phabricator via Openmp-commits]

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

Some minor notes, assuming the tests passes properly after, LGTM



================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:3652
+  /// Fold an attribute of a kerrnel into a constant if possible.
+  /// The value is an attribute in the kernel
+  ChangeStatus foldKernelFnAttribute(Attributor &A, llvm::StringRef Attr) {
----------------



================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:3674
+        return indicatePessimisticFixpoint();
+    }
+
----------------
You should set CurrentAttrValue = NextAttrValue at the end of the loop.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:3679
+      SimplifiedValue =
+          ConstantInt::get(Type::getInt8Ty(Ctx), CurrentAttrValue);
+    }
----------------



================
Comment at: llvm/test/Transforms/OpenMP/get_hardware_num_threads_in_block_fold.ll:6
+%struct.ident_t = type { i32, i32, i32, i32, i8* }
+
+ at G = external global i32
----------------
you need a variable KERNEL_NAME_exec_mode per kernel here. Look at other tests or what clang generates.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106033