This is an archive of the discontinued LLVM Phabricator instance.

[OCaml] Remove all PassManager-related functions
ClosedPublic

Authored by alan on Feb 24 2023, 11:24 AM.

Diff Detail

Event Timeline

alan created this revision.Feb 24 2023, 11:24 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 24 2023, 11:24 AM
Herald added a subscriber: ormris. · View Herald Transcript
alan requested review of this revision.Feb 24 2023, 11:24 AM

In https://reviews.llvm.org/D144741#4151050, I wasn't sure how much heavylifting "pretty much" was doing in "pretty much anything with PassManager in it in the current ocaml bindings will be removed." Is there anything I removed in here that should be kept?

A benefit of removing the PassManager binding code now would that there is less code to review for my patch converting the bindings for OCaml 5.

aeubanks accepted this revision.Feb 24 2023, 12:46 PM

looks right to me, I can land this for you

This revision is now accepted and ready to land.Feb 24 2023, 12:46 PM
nikic accepted this revision.Feb 24 2023, 1:01 PM

LGTM as well

This revision was landed with ongoing or failed builds.Feb 24 2023, 1:14 PM
This revision was automatically updated to reflect the committed changes.
nikic added a comment.Feb 24 2023, 1:18 PM

As a side note, I would recommend requesting commit access as described in https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access.

alan added a comment.Feb 24 2023, 7:17 PM

Thank you for the advice @nikoc, I will consider it. I'm worried that I'm not yet familiar enough with the LLVM collaboration process and may make mistakes. For example, I'm new to Arcanist and worried that I may do something wrong if I'm given permission to run arc land, a command that I've never run before. I also notice that sometimes, the CI for my revisions has a spurious build failure that seems related to my code; should I be worried about those?

On that note, I got an email that I am one of two people possibly responsible for a build failure: https://lab.llvm.org/buildbot/#/builders/178/builds/4032/steps/13/logs/FAIL__lit___shtest-define_py Did I cause this?

@alan Unless the build failure is obviously related to your patch, you can ignore it. If it turns out to be because of your patch after all, a buildbot owner will revert it or contact you. As you're working on the ocaml bindings, it should be relatively obvious which failures are due to your changes and which ones are unrelated/flaky.

llvm/test/Bindings/OCaml/scalar_opts.ml