[PATCH] D138496: [WIP][OMPIRBuilder] Add OpenMPDefaultSimdAlignment field to TargetMachine class

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

The idea is good, we should properly ask the target. That said, don't we need to also provide the type?



================
Comment at: clang/lib/AST/ASTContext.cpp:2509
+      getTargetInfo().getTriple().str(), Target->getTargetOpts().CPU,
+      TargetFeaturesString);
   return SimdAlign;
----------------
Why `.str()` ?


================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:405
                                    InsertPointTy ComputeIP);
+  static unsigned getDefaultTargetSimdAlignment(const std::string &Triple,
+                                                StringRef CPU,
----------------
Documentation please.


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:3070
+  if (!TheTarget) {
+    return {};
+  }
----------------



================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:3078
+  return TgtInfo->getSimdDefaultAlignment();
+}
+
----------------
No `llvm::` this is llvm. Do we really want to create a target machine every time? Are the features different? Can we cache the machine or is it cheap to create?


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

https://reviews.llvm.org/D138496