Page MenuHomePhabricator

alan (Alan)
User

Projects

User does not belong to any projects.

User Details

User Since
Sep 28 2022, 4:27 PM (9 w, 10 h)

Recent Activity

Oct 31 2022

alan updated the diff for D136537: [llvm-ocaml] Assume pointers are at least 2-bit aligned.

Rebase

Oct 31 2022, 6:08 PM · Restricted Project, Restricted Project
alan added inline comments to D136914: [llvm][ocaml] Replace deprecated C functions in OCaml bindings.
Oct 31 2022, 6:06 PM · Restricted Project, Restricted Project
alan updated the diff for D136400: [llvm-ocaml] Migrate from naked pointers to prepare for OCaml 5.

Rebase

Oct 31 2022, 4:14 PM · Restricted Project, Restricted Project

Oct 28 2022

alan added a comment to D136914: [llvm][ocaml] Replace deprecated C functions in OCaml bindings.

I don't have commit permissions to the LLVM repo, so if you need this patch for your own patch, you can commit it yourself.

Oct 28 2022, 2:11 PM · Restricted Project, Restricted Project

Oct 27 2022

alan retitled D136914: [llvm][ocaml] Replace deprecated C functions in OCaml bindings from [llvm][ocaml] Remove or replace OCaml functions with deprecated C counterparts to [llvm][ocaml] Replace deprecated C functions in OCaml bindings.
Oct 27 2022, 11:15 PM · Restricted Project, Restricted Project
alan updated the diff for D136914: [llvm][ocaml] Replace deprecated C functions in OCaml bindings.

Update commit message and refine doc comment

Oct 27 2022, 9:41 PM · Restricted Project, Restricted Project
alan updated the diff for D136914: [llvm][ocaml] Replace deprecated C functions in OCaml bindings.

Replace const_element with aggregate_element

Oct 27 2022, 9:37 PM · Restricted Project, Restricted Project
alan updated the summary of D136914: [llvm][ocaml] Replace deprecated C functions in OCaml bindings.
Oct 27 2022, 9:19 PM · Restricted Project, Restricted Project
alan requested review of D136914: [llvm][ocaml] Replace deprecated C functions in OCaml bindings.
Oct 27 2022, 9:17 PM · Restricted Project, Restricted Project

Oct 25 2022

alan updated the diff for D136537: [llvm-ocaml] Assume pointers are at least 2-bit aligned.

Rebase onto parent patch

Oct 25 2022, 4:06 PM · Restricted Project, Restricted Project
alan updated the diff for D136400: [llvm-ocaml] Migrate from naked pointers to prepare for OCaml 5.

Check for NULL in alloc_temp

Oct 25 2022, 3:59 PM · Restricted Project, Restricted Project

Oct 24 2022

alan updated the diff for D136537: [llvm-ocaml] Assume pointers are at least 2-bit aligned.

Assume all functions may allocate on the OCaml heap

Oct 24 2022, 4:23 PM · Restricted Project, Restricted Project
alan added a comment to D136400: [llvm-ocaml] Migrate from naked pointers to prepare for OCaml 5.

I'm concerned about the diagnostic handler-related functions. An arbitrary OCaml function can be set as the handler, so any LLVM function that invokes the diagnostic handler anywhere may allocate on the OCaml heap, therefore triggering the GC. (This weakens some assumptions that I thought allowed me to omit some instances of CAMLparam in my stacked diff.) What's more, any such LLVM function may throw an OCaml exception, which will unwind the stack, meaning:

Oct 24 2022, 2:25 PM · Restricted Project, Restricted Project
alan added a comment to D136537: [llvm-ocaml] Assume pointers are at least 2-bit aligned.

https://discuss.ocaml.org/t/relaxed-rules-for-binding-a-c-library/10693/ seems to imply that the shortcuts that this diff makes are still safe.

Oct 24 2022, 5:18 AM · Restricted Project, Restricted Project

Oct 23 2022

alan added a comment to D136400: [llvm-ocaml] Migrate from naked pointers to prepare for OCaml 5.

The original binding code on the main branch often skipped using CAMLparamX and CAMLreturn because many of the functions did not manipulate OCaml heap objects. When converting the bindings to eliminate naked pointers, I followed all the rules at https://v2.ocaml.org/manual/intfc.html because I assumed that to_val might allocate on the OCaml heap. Now, I have a stacked diff at https://reviews.llvm.org/D136537 that assumes that all pointers that LLVM provides OCaml are at least 2-bit aligned, and therefore don't require an allocation on the OCaml heap. This diff also:

Oct 23 2022, 12:46 AM · Restricted Project, Restricted Project
alan updated the diff for D136537: [llvm-ocaml] Assume pointers are at least 2-bit aligned.

Convert the rest of the files

Oct 23 2022, 12:37 AM · Restricted Project, Restricted Project

Oct 22 2022

alan updated the diff for D136537: [llvm-ocaml] Assume pointers are at least 2-bit aligned.

Fix missing CAMLreturn causing hanging tests

Oct 22 2022, 11:06 PM · Restricted Project, Restricted Project
alan updated the diff for D136537: [llvm-ocaml] Assume pointers are at least 2-bit aligned.

Add missing CAMLreturn

Oct 22 2022, 9:13 PM · Restricted Project, Restricted Project
alan updated the diff for D136537: [llvm-ocaml] Assume pointers are at least 2-bit aligned.

Don't use CAML macros in iterator functions

Oct 22 2022, 7:13 PM · Restricted Project, Restricted Project
alan added reviewers for D136537: [llvm-ocaml] Assume pointers are at least 2-bit aligned: jberdine, nikic, aeubanks, jrtc27.
Oct 22 2022, 6:51 PM · Restricted Project, Restricted Project
alan added a comment to D136537: [llvm-ocaml] Assume pointers are at least 2-bit aligned.

I'm extremely puzzled by https://github.com/llvm/llvm-project/blob/main/llvm/test/Bindings/OCaml/ext_exc.ml. When I test on OCaml 4.14, all tests pass, but when I test on OCaml 5.0~beta1, this test hangs forever. All tests on the parent patch pass on OCaml 5.0~beta1.

Oct 22 2022, 6:51 PM · Restricted Project, Restricted Project
alan updated the summary of D136537: [llvm-ocaml] Assume pointers are at least 2-bit aligned.
Oct 22 2022, 4:16 PM · Restricted Project, Restricted Project
alan updated subscribers of D136537: [llvm-ocaml] Assume pointers are at least 2-bit aligned.
Oct 22 2022, 4:10 PM · Restricted Project, Restricted Project
alan updated the diff for D136537: [llvm-ocaml] Assume pointers are at least 2-bit aligned.

Add missing return

Oct 22 2022, 3:28 PM · Restricted Project, Restricted Project
alan updated the diff for D136537: [llvm-ocaml] Assume pointers are at least 2-bit aligned.

Testing...

Oct 22 2022, 3:15 PM · Restricted Project, Restricted Project
alan updated subscribers of D136537: [llvm-ocaml] Assume pointers are at least 2-bit aligned.
Oct 22 2022, 1:09 PM · Restricted Project, Restricted Project
alan requested review of D136537: [llvm-ocaml] Assume pointers are at least 2-bit aligned.
Oct 22 2022, 12:56 PM · Restricted Project, Restricted Project
alan retitled D136400: [llvm-ocaml] Migrate from naked pointers to prepare for OCaml 5 from [ocaml-llvm] Migrate from naked pointers in preparation for OCaml 5 to [llvm-ocaml] Migrate from naked pointers to prepare for OCaml 5.
Oct 22 2022, 12:31 PM · Restricted Project, Restricted Project
alan abandoned D136535: [llvm-ocaml] Assume pointers are at least 2-bit aligned.

I wanted to make a stacked diff, but I'm still new to Phabricator and messed it up...

Oct 22 2022, 12:27 PM · Restricted Project, Restricted Project
alan updated the diff for D136400: [llvm-ocaml] Migrate from naked pointers to prepare for OCaml 5.

Rewrite commit message

Oct 22 2022, 12:25 PM · Restricted Project, Restricted Project
alan added a comment to D136400: [llvm-ocaml] Migrate from naked pointers to prepare for OCaml 5.
Oct 22 2022, 12:18 PM · Restricted Project, Restricted Project
alan requested review of D136535: [llvm-ocaml] Assume pointers are at least 2-bit aligned.
Oct 22 2022, 12:08 PM · Restricted Project, Restricted Project
alan added a comment to D136400: [llvm-ocaml] Migrate from naked pointers to prepare for OCaml 5.

For what it's worth, in my local repository, I redefined to_val to the following:

Oct 22 2022, 10:42 AM · Restricted Project, Restricted Project
alan updated the diff for D136400: [llvm-ocaml] Migrate from naked pointers to prepare for OCaml 5.

Remove outdated comments

Oct 22 2022, 10:18 AM · Restricted Project, Restricted Project

Oct 21 2022

alan added a comment to D136400: [llvm-ocaml] Migrate from naked pointers to prepare for OCaml 5.

The original code defined caml_alloc_tuple_uninit, which either called caml_alloc_small or caml_alloc_shr. Then, the original code would initialize the allocation by passing the pointer to the LLVM C API function.

Oct 21 2022, 5:25 PM · Restricted Project, Restricted Project
alan updated the diff for D136400: [llvm-ocaml] Migrate from naked pointers to prepare for OCaml 5.

Replace more returns with CAMLreturns in debuginfo

Oct 21 2022, 5:14 PM · Restricted Project, Restricted Project
alan updated the diff for D136400: [llvm-ocaml] Migrate from naked pointers to prepare for OCaml 5.

Eliminate unsafe uses of caml_alloc_small

Oct 21 2022, 3:48 PM · Restricted Project, Restricted Project
alan added a reviewer for D136400: [llvm-ocaml] Migrate from naked pointers to prepare for OCaml 5: whitequark.
Oct 21 2022, 5:29 AM · Restricted Project, Restricted Project

Oct 20 2022

alan updated the diff for D136400: [llvm-ocaml] Migrate from naked pointers to prepare for OCaml 5.

Wrap data in CAMLlocal1

Oct 20 2022, 6:40 PM · Restricted Project, Restricted Project
alan added reviewers for D136400: [llvm-ocaml] Migrate from naked pointers to prepare for OCaml 5: jberdine, nikic, aeubanks.
Oct 20 2022, 5:21 PM · Restricted Project, Restricted Project
alan requested review of D136400: [llvm-ocaml] Migrate from naked pointers to prepare for OCaml 5.
Oct 20 2022, 5:19 PM · Restricted Project, Restricted Project

Oct 12 2022

alan added a comment to D135842: [llvm-ocaml] Fix arity mismatch in pointer bindings.

I apologize for my sloppy handiwork in my previous patch... I just noticed this mistake.

Oct 12 2022, 7:12 PM · Restricted Project, Restricted Project
alan added reviewers for D135842: [llvm-ocaml] Fix arity mismatch in pointer bindings: aeubanks, jberdine, nikic.
Oct 12 2022, 7:10 PM · Restricted Project, Restricted Project
alan requested review of D135842: [llvm-ocaml] Fix arity mismatch in pointer bindings.
Oct 12 2022, 7:08 PM · Restricted Project, Restricted Project

Oct 11 2022

alan abandoned D135499: [llvm-ocaml] Revise OCaml opaque pointer bindings to match rest of API.

Superseded by https://reviews.llvm.org/rGd226244ae0f5b85d21c9cd23f27f1d7890ea3b4f.

Oct 11 2022, 6:09 PM · Restricted Project, Restricted Project
alan updated the diff for D135524: [llvm-ocaml] Replace all typed pointer functions with opaque pointer functions.

Run clang-format on changes

Oct 11 2022, 2:58 PM · Restricted Project, Restricted Project

Oct 8 2022

alan updated the diff for D135524: [llvm-ocaml] Replace all typed pointer functions with opaque pointer functions.

Consistently use the name [sp] for address space

Oct 8 2022, 7:18 PM · Restricted Project, Restricted Project
alan added a comment to D135499: [llvm-ocaml] Revise OCaml opaque pointer bindings to match rest of API.

I now made https://reviews.llvm.org/D135524, which is an alternative to this patch. For context, see https://discourse.llvm.org/t/i-accidentally-created-a-naming-inconsistency-in-the-ocaml-api/65715. I intend for either one or the other to be merged depending on which my reviewers think is more appropriate.

Oct 8 2022, 7:08 PM · Restricted Project, Restricted Project
alan added a reviewer for D135524: [llvm-ocaml] Replace all typed pointer functions with opaque pointer functions: aeubanks.
Oct 8 2022, 7:07 PM · Restricted Project, Restricted Project
alan added a reviewer for D135499: [llvm-ocaml] Revise OCaml opaque pointer bindings to match rest of API: nikic.
Oct 8 2022, 7:06 PM · Restricted Project, Restricted Project
alan added reviewers for D135524: [llvm-ocaml] Replace all typed pointer functions with opaque pointer functions: jberdine, nikic.
Oct 8 2022, 7:02 PM · Restricted Project, Restricted Project
alan requested review of D135524: [llvm-ocaml] Replace all typed pointer functions with opaque pointer functions.
Oct 8 2022, 7:01 PM · Restricted Project, Restricted Project

Oct 7 2022

alan added a comment to D135499: [llvm-ocaml] Revise OCaml opaque pointer bindings to match rest of API.

According to https://llvm.org/docs/OpaquePointers.html#version-support, LLVM 16 will not support typed pointers. So, if this patch is not going into an LLVM 15 minor or patch release, could one totally remove the old pointer_type and qualified_pointer_type functions and give their names to pointer_type2 and qualified_pointer_type2? However, that involves rewriting the tests, which use the old functions, and I'm not an expert on what all the tests are supposed to check...

Oct 7 2022, 6:46 PM · Restricted Project, Restricted Project
alan added a comment to D135499: [llvm-ocaml] Revise OCaml opaque pointer bindings to match rest of API.

Also, I chose not to use the word "opaque" in the docs because once typed pointers are phased out, *all* pointers will be opaque pointers.

Oct 7 2022, 6:11 PM · Restricted Project, Restricted Project
alan updated the diff for D135499: [llvm-ocaml] Revise OCaml opaque pointer bindings to match rest of API.

Make doc comment of pointer_type2 refer to getUnqual, not get

Oct 7 2022, 5:46 PM · Restricted Project, Restricted Project
alan edited reviewers for D135499: [llvm-ocaml] Revise OCaml opaque pointer bindings to match rest of API, added: aeubanks; removed: nikic.
Oct 7 2022, 4:52 PM · Restricted Project, Restricted Project
alan added a comment to D135499: [llvm-ocaml] Revise OCaml opaque pointer bindings to match rest of API.

Per the discussion on https://discourse.llvm.org/t/i-accidentally-created-a-naming-inconsistency-in-the-ocaml-api/65715, I renamed the pointer_type_in_context function I previously added. Because this function takes an address space as an argument, I decided to rename it to qualified_pointer_type2, rather than pointer_type2, to match the existing qualified_pointer_type function, which also takes an address space as an argument. Then, I added another function, pointer_type2, which uses the opaque pointer API and does not take an address space as an argument, to correspond with the pointer_type function.

Oct 7 2022, 4:47 PM · Restricted Project, Restricted Project
alan added reviewers for D135499: [llvm-ocaml] Revise OCaml opaque pointer bindings to match rest of API: jberdine, nikic.
Oct 7 2022, 4:43 PM · Restricted Project, Restricted Project
alan requested review of D135499: [llvm-ocaml] Revise OCaml opaque pointer bindings to match rest of API.
Oct 7 2022, 4:18 PM · Restricted Project, Restricted Project

Oct 2 2022

alan added a comment to D134916: [llvm-ocaml] Add binding for constructing opaque pointers.

Thank you for the approval! I do not believe I have permissions to merge, as I just joined Phabricator to make this patch. According to this: https://www.llvm.org/docs/Phabricator.html#committing-a-change

Oct 2 2022, 6:11 PM · Restricted Project, Restricted Project
alan added a reviewer for D134916: [llvm-ocaml] Add binding for constructing opaque pointers: jberdine.
Oct 2 2022, 12:33 PM · Restricted Project, Restricted Project

Oct 1 2022

alan added a comment to D134916: [llvm-ocaml] Add binding for constructing opaque pointers.

Just rearranged the test order at the bottom to match the order of test definition...

Oct 1 2022, 8:58 AM · Restricted Project, Restricted Project
alan updated the diff for D134916: [llvm-ocaml] Add binding for constructing opaque pointers.

[llvm-ocaml] Add binding for constructing opaque pointers

Oct 1 2022, 8:57 AM · Restricted Project, Restricted Project

Sep 30 2022

alan added a comment to D134916: [llvm-ocaml] Add binding for constructing opaque pointers.

I just updated the diff to remove the PointerType::isOpaque binding I added. The only reason I added that was to write a test case checking if a pointer was opaque, which I had *misunderstood* and written incorrectly, as @nikic pointed out. As @aeubanks pointed out, users should assume that all pointers are opaque now (as LLVM is migrating to opaque pointers), so the isOpaque binding perhaps should not be added.

Sep 30 2022, 9:32 PM · Restricted Project, Restricted Project
alan updated the diff for D134916: [llvm-ocaml] Add binding for constructing opaque pointers.

[llvm-ocaml] Add binding for constructing opaque pointers

Sep 30 2022, 9:27 PM · Restricted Project, Restricted Project
alan added a comment to D134916: [llvm-ocaml] Add binding for constructing opaque pointers.

Also, as I update this patch using the "make a new branch and squash merge" method, my reviewers' inline comments still appear, but are shown on the "new" version of the patch where I fix the issue, which creates a confusing context. I want to know if I'm doing the Phabricator workflow correctly, or if there is something else I should be doing.

Sep 30 2022, 6:15 PM · Restricted Project, Restricted Project
alan updated the diff for D134916: [llvm-ocaml] Add binding for constructing opaque pointers.

[llvm-ocaml] Add binding for constructing opaque pointers

Sep 30 2022, 5:59 PM · Restricted Project, Restricted Project
alan added a comment to D134916: [llvm-ocaml] Add binding for constructing opaque pointers.

It turns out that @nikic was correct to be fishy by pointing out that a "test" I added should not have passed! I figured out that I added the test function, but I had not added it to the list of tests at the bottom of the file, so it never ran. When I did run it, the test did indeed fail. I've updated the diff to fix this, as well as to add the missing word "space" in a doc comment that was pointed out.

Sep 30 2022, 5:56 PM · Restricted Project, Restricted Project
alan updated the diff for D134916: [llvm-ocaml] Add binding for constructing opaque pointers.

[llvm-ocaml] Add binding for constructing opaque pointers

Sep 30 2022, 5:47 PM · Restricted Project, Restricted Project
alan added a comment to D134916: [llvm-ocaml] Add binding for constructing opaque pointers.
Sep 30 2022, 5:12 PM · Restricted Project, Restricted Project
alan added a comment to D134916: [llvm-ocaml] Add binding for constructing opaque pointers.

I've updated the patch by making a new feature branch and squash merging the old one into the new one. Now, both my commits are included in the diff. Was there a better way for me to have done this?

Sep 30 2022, 6:48 AM · Restricted Project, Restricted Project
alan updated the diff for D134916: [llvm-ocaml] Add binding for constructing opaque pointers.

[llvm-ocaml] Add binding for constructing opaque pointers

Sep 30 2022, 6:47 AM · Restricted Project, Restricted Project
alan added a comment to D134916: [llvm-ocaml] Add binding for constructing opaque pointers.

I think your update overwrite your previous changes, which added pointer_type_in_context? Now you're just adding pointer_type_is_opaque (but the tests still use the other function as well).

Sep 30 2022, 6:32 AM · Restricted Project, Restricted Project

Sep 29 2022

alan added a comment to D134916: [llvm-ocaml] Add binding for constructing opaque pointers.

Hi, I am a first-time contributor to LLVM. I just requested a review from the last two people to make commits to the OCaml LLVM bindings, because I wasn't sure who the code owner was. For context, see https://discourse.llvm.org/t/guidance-for-doing-my-first-contribution/65590/3. Please let me know if I'm doing anything wrong!

Sep 29 2022, 10:57 PM · Restricted Project, Restricted Project
alan added reviewers for D134916: [llvm-ocaml] Add binding for constructing opaque pointers: aeubanks, nikic.
Sep 29 2022, 10:56 PM · Restricted Project, Restricted Project
alan updated the diff for D134916: [llvm-ocaml] Add binding for constructing opaque pointers.

[llvm-ocaml] Add binding for llvm::PointerType::isOpaque

Sep 29 2022, 10:49 PM · Restricted Project, Restricted Project
alan requested review of D134916: [llvm-ocaml] Add binding for constructing opaque pointers.
Sep 29 2022, 3:23 PM · Restricted Project, Restricted Project