[PATCH] D136537: [llvm-ocaml] Assume pointers are at least 2-bit aligned
Alan via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 14 11:43:03 PST 2023
alan added inline comments.
================
Comment at: llvm/bindings/ocaml/llvm/llvm_ocaml.c:250-252
+ CAMLparam1(Value);
CAMLreturn(to_val(LLVMCreateEnumAttribute(Context_val(C), Int_val(Kind),
Int64_val(Value))));
----------------
jberdine wrote:
> CAMLparam and CAMLreturn do not need to be added (by the composition of this diff and its parent) here, right? There are other functions that seem to be in a similar situation, but asking about one example to check my understanding.
In this diff, I decided to take some liberties by not registering a value with CAMLparam if it is known to not be a GC'd block (for example, if it is an int, or a pointer from LLVM that is represented by setting the low bit). The original bindings on main take this type of liberty a lot by omitting CAMlparam entirely in many places. In my parent diff, I added the CAMlparams and registered all values with CAMlparam, including ones that the GC doesn't need to track, to be safe, but here, I decided to relax which values I registered.
Is this a good choice? Registering all values probably makes the code clearer and reduces room for error (e.g. forgetting to register something), choosing not to register things like ints and 2-bit aligned LLVM pointers with the low bit set probably slightly increases performance.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136537/new/
https://reviews.llvm.org/D136537
More information about the llvm-commits
mailing list