[PATCH] D65138: [OCaml] Add LLVMInternalizePredicateBindings

[whitequark via Phabricator via llvm-commits]

whitequark added inline comments.


================
Comment at: bindings/ocaml/transforms/ipo/llvm_ipo.mli:76
+    If predicate returns [true], that symbol is preserved.
+    NOT THREAD SAFE! *)
+val add_internalize_predicate
----------------
kren1 wrote:
> whitequark wrote:
> > I don't think we make any claims of thread safety for any other API. Why a warning on this one?
> I felt it's necessary to warn the user that the predicate is not passed by value, but its reference is stored in a global variable. This kind of seemed like the most concise way of saying that. Would you prefer it rephrased to more closely match the above or just removed?
Wait. That's actually worse, because it's not *non-thread-safe*, it's not *reentrant*. Which probably doesn't matter for this specific function, but would be good to solve in general.

I haven't written any really complex OCaml/C interop code in a few years, so my skills here are a bit rusty... could you please look into a better way to do this? If not, no worries, I'll refresh my memory and dig into it myself, but it might take some weeks to get around to this.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65138