Page MenuHomePhabricator

vaivaswatha (Vaivaswatha Nagaraj)
User

Projects

User does not belong to any projects.

User Details

User Since
Jan 15 2015, 10:28 PM (329 w, 16 h)

Recent Activity

Apr 3 2021

vaivaswatha added a comment to D99475: [OCaml] Omit unnecessary GC root registrations.

See https://github.com/jberdine/llvm-project/tree/ocaml for this stack on github.

For the init code, I just mean to add that Gc.set call somewhere in your code that uses the LLVM bindings before you do much with Llvm.

Apr 3 2021, 2:23 AM · Restricted Project

Mar 31 2021

vaivaswatha added a comment to D99475: [OCaml] Omit unnecessary GC root registrations.

I have been testing this stack of diffs with a sort of stress test that involves running a system that consumes bitcode and uses the ocaml api to traverse and translate it. The testing has used a range of OCaml GC settings in order to make the GC work much harder and trigger much more often than usual (e.g. Gc.set {(Gc.get ()) with minor_heap_size= 1024; space_overhead= 20}). The reasoning here is that if the bindings are not playing nice with the GC, then there is a higher chance to uncover such issues if the GC runs much more often. This testing has not revealed any issues.

@vaivaswatha if it is not too much overhead in your application, it would be helpful if you could arc patch these diffs, add a call to Gc.set as above in your initialization code, and just run your application to see if there are any crashes.

Mar 31 2021, 9:34 PM · Restricted Project

Mar 29 2021

vaivaswatha accepted D99474: [OCaml] Code simplification using string allocation functions.
Mar 29 2021, 10:14 AM · Restricted Project
vaivaswatha accepted D99473: [OCaml] Code simplification using option allocation functions.
Mar 29 2021, 10:13 AM · Restricted Project
vaivaswatha accepted D99475: [OCaml] Omit unnecessary GC root registrations.

Similar to my comment in https://reviews.llvm.org/D99472, I don't understand this a 100%, but it seems sane to me.

Mar 29 2021, 10:05 AM · Restricted Project
vaivaswatha accepted D99472: [OCaml] Minor optimizations by avoiding double initialization.

As far as I can see, this looks fine, but I'm far from an expert on OCaml internals. It would be greatly useful if someone else who understands OCaml internals can take a look, but I don't know if it's easy finding such a reviewer here. So if you're confident and the code has been tested well on your end, I suppose it's okay to commit this.

Mar 29 2021, 10:01 AM · Restricted Project
vaivaswatha accepted D99471: [OCaml] Fix unsafe uses of Store_field.

This fixes a potentially hard-to-discover bug ! The last thing that a user of the OCaml API would think of is "oh this function uses Store_field to initialize fields of blocks allocated with caml_alloc_small and that could lead to a hard to trace GC bug".

Mar 29 2021, 9:53 AM · Restricted Project

Mar 28 2021

vaivaswatha accepted D99477: [NFC][OCaml] Reformat to clean up following CAMLprim removal.
Mar 28 2021, 7:47 PM · Restricted Project
vaivaswatha accepted D99476: [NFC][OCaml] Remove vestigial CAMLprim declarations.
Mar 28 2021, 7:46 PM · Restricted Project

Mar 27 2021

vaivaswatha committed rG11f59c5457d5: [OCaml][Test] Fix and enable debuginfo.ml test (authored by vaivaswatha).
[OCaml][Test] Fix and enable debuginfo.ml test
Mar 27 2021, 6:14 PM
vaivaswatha closed D99450: [OCaml][Test] Fix and enable debuginfo.ml test..
Mar 27 2021, 6:13 PM · Restricted Project

Mar 26 2021

vaivaswatha set the repository for D99450: [OCaml][Test] Fix and enable debuginfo.ml test. to rG LLVM Github Monorepo.
Mar 26 2021, 9:29 PM · Restricted Project
vaivaswatha requested review of D99450: [OCaml][Test] Fix and enable debuginfo.ml test..
Mar 26 2021, 7:10 PM · Restricted Project
vaivaswatha committed rG2218bc69d1ff: [OCaml][DebugInfo][Test] Disable debuginfo tests as they fail on some machines (authored by vaivaswatha).
[OCaml][DebugInfo][Test] Disable debuginfo tests as they fail on some machines
Mar 26 2021, 10:27 AM
vaivaswatha added a comment to D99403: [OCaml][DebugInfo] Add tests for the recently added debug info API.

Option seems to not work on the buildbot machines. So I ended up replacing that with a match

Mar 26 2021, 10:18 AM · Restricted Project
vaivaswatha committed rGa502ac383e03: [OCaml][Test] Do not use Option, expand using match (authored by vaivaswatha).
[OCaml][Test] Do not use Option, expand using match
Mar 26 2021, 10:14 AM
vaivaswatha committed rGc244cd72172c: [OCaml][DebugInfo] Add tests for debug info API (authored by vaivaswatha).
[OCaml][DebugInfo] Add tests for debug info API
Mar 26 2021, 9:56 AM
vaivaswatha closed D99403: [OCaml][DebugInfo] Add tests for the recently added debug info API.
Mar 26 2021, 9:55 AM · Restricted Project
vaivaswatha accepted D99420: [NFC][OCaml] Resolve a couple more compilation warnings.
Mar 26 2021, 9:36 AM · Restricted Project
vaivaswatha accepted D99393: [OCaml] Fix a possible crash in llvm_struct_name.
Mar 26 2021, 3:15 AM · Restricted Project
vaivaswatha updated the summary of D99403: [OCaml][DebugInfo] Add tests for the recently added debug info API.
Mar 26 2021, 2:28 AM · Restricted Project
vaivaswatha updated the diff for D99403: [OCaml][DebugInfo] Add tests for the recently added debug info API.

Remove #include <stdio.h> that was inadvertently added.

Mar 26 2021, 2:23 AM · Restricted Project
vaivaswatha requested review of D99403: [OCaml][DebugInfo] Add tests for the recently added debug info API.
Mar 26 2021, 2:18 AM · Restricted Project

Mar 25 2021

vaivaswatha added inline comments to D99393: [OCaml] Fix a possible crash in llvm_struct_name.
Mar 25 2021, 8:06 PM · Restricted Project
vaivaswatha accepted D99392: [NFC][OCaml] Resolve const and unsigned compilation warnings.
Mar 25 2021, 7:29 PM · Restricted Project
vaivaswatha accepted D99391: [NFC][OCaml] Simplify llvm_global_initializer using ptr_to_option.
Mar 25 2021, 7:28 PM · Restricted Project

Mar 20 2021

vaivaswatha committed rGf860187ea6e9: [OCaml] Add (get/set)_module_identifer functions (authored by vaivaswatha).
[OCaml] Add (get/set)_module_identifer functions
Mar 20 2021, 8:37 AM
vaivaswatha closed D98851: [OCaml] Add (get/set)_module_identifer functions.
Mar 20 2021, 8:37 AM · Restricted Project

Mar 19 2021

vaivaswatha added inline comments to D65195: [OCaml] Handle nullptr in Llvm.global_initializer.
Mar 19 2021, 8:16 AM · Restricted Project
vaivaswatha added inline comments to D98851: [OCaml] Add (get/set)_module_identifer functions.
Mar 19 2021, 7:02 AM · Restricted Project
vaivaswatha added inline comments to D98851: [OCaml] Add (get/set)_module_identifer functions.
Mar 19 2021, 6:55 AM · Restricted Project
vaivaswatha added inline comments to D98851: [OCaml] Add (get/set)_module_identifer functions.
Mar 19 2021, 6:44 AM · Restricted Project

Mar 18 2021

vaivaswatha added inline comments to D98851: [OCaml] Add (get/set)_module_identifer functions.
Mar 18 2021, 6:23 PM · Restricted Project
vaivaswatha updated the diff for D98851: [OCaml] Add (get/set)_module_identifer functions.

Whitespace changes

Mar 18 2021, 9:32 AM · Restricted Project
vaivaswatha updated the diff for D98851: [OCaml] Add (get/set)_module_identifer functions.

Use mlsize_t for cstr_to_string as suggested in the review.

Mar 18 2021, 9:27 AM · Restricted Project
vaivaswatha requested review of D98851: [OCaml] Add (get/set)_module_identifer functions.
Mar 18 2021, 3:52 AM · Restricted Project

Mar 16 2021

vaivaswatha added a comment to D90831: DebugInfo support for OCaml bindings.

@jberdine: after commit, I found this failure emailed to me llvm_ocaml.c:function cstr_to_string: error: undefined reference to 'caml_alloc_initialized_string'. Do you happen to know how I can fix it, because it works on my Ubuntu 20.04 system. What OS do you have? (because you reported that it worked too). A link to one of the build fails I got over email: https://lab.llvm.org/buildbot/#/builders/16/builds/7919

I've now switched to using the following code instead in cstr_to_string:

if (Str) {
  String = caml_alloc_string(Len);
  memcpy(String_val(Str), Str, Len);
} else {
  String = caml_alloc_string(0);
}
Mar 16 2021, 11:22 PM · Restricted Project
vaivaswatha added a comment to D90831: DebugInfo support for OCaml bindings.

@jberdine: after commit, I found this failure emailed to me llvm_ocaml.c:function cstr_to_string: error: undefined reference to 'caml_alloc_initialized_string'. Do you happen to know how I can fix it, because it works on my Ubuntu 20.04 system. What OS do you have? (because you reported that it worked too). A link to one of the build fails I got over email: https://lab.llvm.org/buildbot/#/builders/16/builds/7919

Mar 16 2021, 11:04 PM · Restricted Project
vaivaswatha committed rGf7be9db6220c: [OCaml] Fix buildbot failure in OCaml tests (authored by vaivaswatha).
[OCaml] Fix buildbot failure in OCaml tests
Mar 16 2021, 11:02 PM
vaivaswatha added a comment to D90831: DebugInfo support for OCaml bindings.

@jberdine: after commit, I found this failure emailed to me llvm_ocaml.c:function cstr_to_string: error: undefined reference to 'caml_alloc_initialized_string'. Do you happen to know how I can fix it, because it works on my Ubuntu 20.04 system. What OS do you have? (because you reported that it worked too). A link to one of the build fails I got over email: https://lab.llvm.org/buildbot/#/builders/16/builds/7919

Mar 16 2021, 10:31 PM · Restricted Project
vaivaswatha committed rG506df1bbfd16: [OCaml] DebugInfo support for OCaml bindings (authored by vaivaswatha).
[OCaml] DebugInfo support for OCaml bindings
Mar 16 2021, 9:47 PM
vaivaswatha closed D90831: DebugInfo support for OCaml bindings.
Mar 16 2021, 9:46 PM · Restricted Project
vaivaswatha added a comment to D90831: DebugInfo support for OCaml bindings.

Thanks @vaivaswatha for dealing with all my nitpicking! Yes, I've made a final pass over this and it looks good. I'd recheck the build before committing given the CI build failure.

Mar 16 2021, 12:17 PM · Restricted Project
vaivaswatha added a comment to D90831: DebugInfo support for OCaml bindings.

Thank you @jberdine for the detailed review and helping through this patch. Shall I commit it?

Mar 16 2021, 11:27 AM · Restricted Project
vaivaswatha updated the diff for D90831: DebugInfo support for OCaml bindings.

I've updated the patch addressing some of your comments. In particular, I didn't add a reference to the C-API to the functions where I had already provided the explanations inline (and the C-API comment doesn't add more value).

Mar 16 2021, 10:46 AM · Restricted Project
vaivaswatha committed rGfe990ee81596: [Docs] Mention linking to reviews page when committing (authored by vaivaswatha).
[Docs] Mention linking to reviews page when committing
Mar 16 2021, 10:35 AM
vaivaswatha closed D98695: [Docs] Suggest mentioning review page in the commit message.
Mar 16 2021, 10:35 AM · Restricted Project
vaivaswatha added a comment to D98695: [Docs] Suggest mentioning review page in the commit message.

Actually, this is mentioned in a different document: https://www.llvm.org/docs/Phabricator.html#committing-a-change

I agree this isn't perfect and I think your change is worth doing. Can you also add a link to the URL above?

Something like "add a link to its review page, as shown in [Phab link]".

Thanks!

Mar 16 2021, 5:12 AM · Restricted Project
vaivaswatha updated the diff for D98695: [Docs] Suggest mentioning review page in the commit message.

Add missing fullstop.

Mar 16 2021, 5:11 AM · Restricted Project
vaivaswatha updated the diff for D98695: [Docs] Suggest mentioning review page in the commit message.

Link to Phabricator page that mentions the same topic.

Mar 16 2021, 5:09 AM · Restricted Project
vaivaswatha added reviewers for D98695: [Docs] Suggest mentioning review page in the commit message: rengolin, silvas.
Mar 16 2021, 4:30 AM · Restricted Project
vaivaswatha requested review of D98695: [Docs] Suggest mentioning review page in the commit message.
Mar 16 2021, 4:21 AM · Restricted Project

Mar 15 2021

vaivaswatha updated the diff for D90831: DebugInfo support for OCaml bindings.

Fix clang-tidy warning on header guard name convention.

Mar 15 2021, 11:43 PM · Restricted Project
vaivaswatha updated the diff for D90831: DebugInfo support for OCaml bindings.

@jberdine: I have now updated the patch with documentation for the newly added APIs, inline. Further:

Mar 15 2021, 11:11 PM · Restricted Project
vaivaswatha added inline comments to D90831: DebugInfo support for OCaml bindings.
Mar 15 2021, 9:23 AM · Restricted Project
vaivaswatha added inline comments to D90831: DebugInfo support for OCaml bindings.
Mar 15 2021, 8:31 AM · Restricted Project
vaivaswatha updated the diff for D90831: DebugInfo support for OCaml bindings.

Thank you very much @jberdine . I've incorporated all of your suggestions with this latest diff. I haven't yet added the documentation and will do that by tomorrow. Please have a final look at the latest changes so that with my update tomorrow it should only be comments over each function and (hopefully) no code changes.

Mar 15 2021, 8:31 AM · Restricted Project
vaivaswatha added a comment to D90831: DebugInfo support for OCaml bindings.

@jberdine I have addressed all your comments, except for the one on stricter type aliases for Metadata.

Is this what you were working toward and ran into problems with?

No, I only attempted something simpler along the lines of your experimental change. Nothing so much as having conversion functions between them.

I need to think what's the best way to model the C++ type hierarchy for Metadata here in OCaml. For example, there is llvalue and llbasicblock already in Llvml.ml, but they just rely on, internally in Core.cpp, cast<> throwing a runtime error (at least that's what I understood of it).

that would optimally be implemented with something like a dyn_cast

We don't have dyn_cast available in the C-API (I couldn't find any uses of it). So we might need to reestablish the subtyping from scratch using the MetadataKind enum values. I'm not sure that will be easy to maintain though. I can give it a try.

Mar 15 2021, 3:17 AM · Restricted Project
vaivaswatha added a comment to D90831: DebugInfo support for OCaml bindings.

The types that we'll need to model include, at least the following:

Mar 15 2021, 3:12 AM · Restricted Project
vaivaswatha updated the diff for D90831: DebugInfo support for OCaml bindings.

Address all but one review comment.

Mar 15 2021, 12:14 AM · Restricted Project
vaivaswatha added a comment to D90831: DebugInfo support for OCaml bindings.

@jberdine I have addressed all your comments, except for the one on stricter type aliases for Metadata.

Mar 15 2021, 12:13 AM · Restricted Project

Mar 14 2021

vaivaswatha updated the diff for D90831: DebugInfo support for OCaml bindings.

Thank you very much @jberdine for the review, especially for the ready to incorporate changes in your fork (which I just cherry-picked).

Mar 14 2021, 1:49 AM · Restricted Project
vaivaswatha accepted D98578: [OCaml] Add missing TypeKinds, Opcode, and AtomicRMWBinOps.
Mar 14 2021, 1:06 AM · Restricted Project

Mar 13 2021

vaivaswatha accepted D98593: [OCaml][test] Fix Bindings/OCaml/executionengine.ml test.
Mar 13 2021, 7:49 PM · Restricted Project

Nov 23 2020

vaivaswatha added a reviewer for D90831: DebugInfo support for OCaml bindings: CodaFi.
Nov 23 2020, 3:07 AM · Restricted Project

Nov 15 2020

vaivaswatha added a comment to D90831: DebugInfo support for OCaml bindings.

It's been more than a week, so a gentile reminder ping !

Nov 15 2020, 10:15 PM · Restricted Project

Nov 5 2020

vaivaswatha added a reviewer for D90831: DebugInfo support for OCaml bindings: jberdine.
Nov 5 2020, 3:16 AM · Restricted Project
vaivaswatha updated subscribers of D90831: DebugInfo support for OCaml bindings.

@jberdine just pinged me about his earlier attempt to do this, here's a reference: https://reviews.llvm.org/D60902.

Nov 5 2020, 3:15 AM · Restricted Project
vaivaswatha updated the diff for D90831: DebugInfo support for OCaml bindings.

Mention both CMAKE_INSTALL_PREFIX and LLVM_OCAML_INSTALL_PATH in the OCaml bindings README

Nov 5 2020, 1:40 AM · Restricted Project
vaivaswatha requested review of D90831: DebugInfo support for OCaml bindings.
Nov 5 2020, 12:57 AM · Restricted Project

Jan 25 2017

vaivaswatha abandoned D20421: [TargetLibraryInfo] Fix signature recognition of some lib functions..

This is already fixed on trunk.

Jan 25 2017, 4:04 AM

May 19 2016

vaivaswatha retitled D20421: [TargetLibraryInfo] Fix signature recognition of some lib functions. from to [TargetLibraryInfo] Fix signature recognition of some lib functions..
May 19 2016, 3:23 AM

Apr 26 2016

vaivaswatha committed rL267671: [Cloning] cloneLoopWithPreheader(): add assert to ensure no sub-loops.
[Cloning] cloneLoopWithPreheader(): add assert to ensure no sub-loops
Apr 26 2016, 10:31 PM
vaivaswatha closed D15922: [Cloning] cloneLoopWithPreheader(): add assert to ensure no sub-loops by committing rL267671: [Cloning] cloneLoopWithPreheader(): add assert to ensure no sub-loops.
Apr 26 2016, 10:31 PM
vaivaswatha updated the diff for D15922: [Cloning] cloneLoopWithPreheader(): add assert to ensure no sub-loops.

Updating based on review comments

Apr 26 2016, 9:54 PM
vaivaswatha retitled D15922: [Cloning] cloneLoopWithPreheader(): add assert to ensure no sub-loops from [Cloning] rename cloneLoopWithPreheader() and add assert to ensure no sub-loops to [Cloning] cloneLoopWithPreheader(): add assert to ensure no sub-loops.
Apr 26 2016, 9:21 PM
vaivaswatha updated the diff for D15922: [Cloning] cloneLoopWithPreheader(): add assert to ensure no sub-loops.

Updating based on review comments.

Apr 26 2016, 9:20 PM
vaivaswatha added a comment to D15922: [Cloning] cloneLoopWithPreheader(): add assert to ensure no sub-loops.

Upon reflection, cloning an outer loop is something we'll want to be able to do at some point. Why don't we just add the assertion for now, and then fix the function once we have a concrete use case?

Apr 26 2016, 8:48 PM

Apr 18 2016

vaivaswatha updated the diff for D15922: [Cloning] cloneLoopWithPreheader(): add assert to ensure no sub-loops.

Updating based on review comments

Apr 18 2016, 9:54 PM

Apr 16 2016

vaivaswatha updated the diff for D15922: [Cloning] cloneLoopWithPreheader(): add assert to ensure no sub-loops.

Updating based on review comments

Apr 16 2016, 9:17 PM

Apr 13 2016

vaivaswatha retitled D15922: [Cloning] cloneLoopWithPreheader(): add assert to ensure no sub-loops from [NOP][Cloning] Add comment to cloneLoopWithPreheader() mentioning that it does not update LoopInfo for sub-loops. to [Cloning] rename cloneLoopWithPreheader() and add assert to ensure no sub-loops.
Apr 13 2016, 5:09 AM
vaivaswatha updated the diff for D15922: [Cloning] cloneLoopWithPreheader(): add assert to ensure no sub-loops.

[Cloning] Add assert to ensure no sub-loops for loop being cloned. Also rename
function to reflect its inability to clone sub-loops. (It can clone sub-loops,
but does not correctly update LoopInfo).

Apr 13 2016, 5:07 AM

Apr 11 2016

vaivaswatha added a comment to D15922: [Cloning] cloneLoopWithPreheader(): add assert to ensure no sub-loops.

Hey sorry I'd forgotten about this patch. I'll work on it and resubmit in a day or two.

Apr 11 2016, 7:36 PM

Feb 15 2016

vaivaswatha added a comment to D17031: Control Structure Analysis.

@joker.eph Thank you for the comments. I'll be happy to work on them (and take the patch to a good state for committing). However at the moment I'm looking for a broader approval that the code would be useful to the community and it would get in after its taken to a good state. I don't want to spend a lot of time reworking it and finally finding it to be not-useful and it not going in.

Feb 15 2016, 6:28 PM

Feb 9 2016

vaivaswatha retitled D17031: Control Structure Analysis from to Control Structure Analysis.
Feb 9 2016, 8:34 AM

Jan 14 2016

vaivaswatha added a comment to D16140: [GlobalsAA] Relax condition in checking globals as args to functions.

Thanks James for the review. Also you thank you for suggesting the original patch.

Jan 14 2016, 12:51 AM
vaivaswatha committed rL257750: [GlobalsAA] Relax condition in checking globals as args to functions.
[GlobalsAA] Relax condition in checking globals as args to functions
Jan 14 2016, 12:50 AM
vaivaswatha closed D16140: [GlobalsAA] Relax condition in checking globals as args to functions.
Jan 14 2016, 12:50 AM

Jan 13 2016

vaivaswatha retitled D16140: [GlobalsAA] Relax condition in checking globals as args to functions from to [GlobalsAA] Relax condition in checking globals as args to functions.
Jan 13 2016, 2:42 AM

Jan 12 2016

vaivaswatha added a reviewer for D15922: [Cloning] cloneLoopWithPreheader(): add assert to ensure no sub-loops: anemet.
Jan 12 2016, 7:46 PM
vaivaswatha added a comment to D15922: [Cloning] cloneLoopWithPreheader(): add assert to ensure no sub-loops.

Ping !

Jan 12 2016, 7:45 PM

Jan 6 2016

vaivaswatha retitled D15922: [Cloning] cloneLoopWithPreheader(): add assert to ensure no sub-loops from to [NOP][Cloning] Add comment to cloneLoopWithPreheader() mentioning that it does not update LoopInfo for sub-loops..
Jan 6 2016, 5:58 AM
vaivaswatha abandoned D15665: GlobalsAA: InaccessibleMemOnly does not mean ReadNone..
Jan 6 2016, 5:37 AM
vaivaswatha abandoned D15777: [GlobalOpt] Globals used only in "main" can more easily be localized.
Jan 6 2016, 5:37 AM
vaivaswatha added a comment to D15919: Revert "GlobalsAA: Take advantage of ArgMemOnly, InaccessibleMemOnly and InaccessibleMemOrArgMemOnly attributes".

Thanks for doing this.

Jan 6 2016, 4:10 AM

Jan 4 2016

vaivaswatha added a comment to D15665: GlobalsAA: InaccessibleMemOnly does not mean ReadNone..

I'm really not sure inaccessible memonly belongs in GlobalsAA at all. Particularly since we haven't added even basis cases to BasicAA.

In general, I think we need to be really careful when implementing this. It's fair to say that the call doesn't modify or read any *particular* LLVM value, but I'm not sure our aliasing model is correct if we say that it can't read *any* LLVM value. In particular, as you point out, inaccessiblememonly may still be modifying memory and have memory dependence.

Jan 4 2016, 6:38 PM
vaivaswatha updated the diff for D15665: GlobalsAA: InaccessibleMemOnly does not mean ReadNone..

Updating comment and test case as per review comment.

Jan 4 2016, 6:33 PM

Dec 25 2015

vaivaswatha retitled D15777: [GlobalOpt] Globals used only in "main" can more easily be localized from to [GlobalOpt] Globals used only in "main" can more easily be localized.
Dec 25 2015, 6:54 AM

Dec 21 2015

vaivaswatha added a comment to D15665: GlobalsAA: InaccessibleMemOnly does not mean ReadNone..

Ping !

Dec 21 2015, 6:45 PM

Dec 19 2015

vaivaswatha retitled D15665: GlobalsAA: InaccessibleMemOnly does not mean ReadNone. from to GlobalsAA: InaccessibleMemOnly does not mean ReadNone..
Dec 19 2015, 12:56 AM