Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Phabricator shutdown timeline

kmod (Kevin Modzelewski)
User

Projects

User does not belong to any projects.

User Details

User Since
Mar 21 2014, 5:03 PM (497 w, 4 d)

Recent Activity

Oct 17 2014

kmod added a comment to D5634: Enable invoking the patchpoint intrinsic.

Ok I rebased onto r220027, but the test is passing for me. Not that that's really a good sign for the test's robustness, but I'm curious now why it's failing. I noticed that I had used a different triple than all of the other tests used so I fixed that -- I wonder if that makes it pass for you? I'm assuming you're running this on a mac so that an incorrectly-specified triple could cause issues since this assumes ELF.

Oct 17 2014, 2:16 AM
kmod updated the diff for D5634: Enable invoking the patchpoint intrinsic.

Try to make the test a little bit less brittle.

Oct 17 2014, 2:13 AM
kmod updated the diff for D5634: Enable invoking the patchpoint intrinsic.

Try to fix the target triple and make the intent of the test more clear

Oct 17 2014, 1:58 AM

Oct 16 2014

kmod updated the diff for D5634: Enable invoking the patchpoint intrinsic.

Update based on review comments

Oct 16 2014, 2:28 PM
kmod added a comment to D5657: Reduce code duplication between patchpoint and non-patchpoint lowering. NFC..

No commit access, if it looks good I'd appreciate if you could commit it :)

Oct 16 2014, 1:56 PM

Oct 15 2014

kmod added a comment to D5683: Statepoint infrastructure for garbage collection.

Sorry yes, I am comparing this approach to gc.root; it seems like gc.root and statepoints are similar in that they both take a spill+reload "reduce the code generator's ability to use copies" approach as compared to a hypothetical "track all copies so that they can be updated in place". It seems like gc.root provides much of the same functionality as statepoint -- gc.root definitely should be able to support a relocating GC as well, and I guess I haven't heard of it being "fundamentally broken" outside of a late-safepoint-placement strategy. So far the arguments I've seen for statepoints over gc.root are

  • easier to save roots in callee-save registers
  • easier to automatically generate gc annotations on arbitrary IR, such as post-compiler-optimizations.
Oct 15 2014, 11:48 PM
kmod updated the diff for D5657: Reduce code duplication between patchpoint and non-patchpoint lowering. NFC..

Not sure how '_' got added

Oct 15 2014, 8:24 PM
kmod updated the diff for D5657: Reduce code duplication between patchpoint and non-patchpoint lowering. NFC..

Address review comments

Oct 15 2014, 8:21 PM
kmod added a comment to D5634: Enable invoking the patchpoint intrinsic.

Ok I will update the patch, but I'm not sure how you mean to not thread
through the ImmutableCallSite, since I thought InvokeInst is not a subclass
of CallInst?

Oct 15 2014, 1:12 PM

Oct 14 2014

kmod added a comment to D5683: Statepoint infrastructure for garbage collection.

I think a change like this might be more compelling if you could give more detail on how it would actually help (I can't find the detail I'm looking for in your blog posts). It seems like the value of this patch is that it will work with late safepoint placement, but it'd be nice to see some examples of cases where late safepoint placement gives you something that early safepoint placement (ie by the frontend) doesn't. It kind of feels like either approach will work well with only non-gc values, and neither approach will be able to do much optimization when you do function calls. I'm not trying to claim that that's necessarily true, but it'd be easier to understand your point if there was some example IR.

Oct 14 2014, 7:01 PM
kmod added a comment to D5634: Enable invoking the patchpoint intrinsic.

Ok, I rebased onto trunk. One thing I failed to mention is that this patch depends on another that I sent out, D5657, since I wanted to separate some refactoring changes from the actual invoke-supporting change.

Oct 14 2014, 3:00 PM
kmod updated the diff for D5634: Enable invoking the patchpoint intrinsic.

Rebase onto trunk (r219719)

Oct 14 2014, 2:54 PM

Oct 11 2014

kmod updated the diff for D5634: Enable invoking the patchpoint intrinsic.

Fix formatting

Oct 11 2014, 2:11 AM

Oct 9 2014

kmod updated the diff for D5696: Add llvm.experimental.patchpoint.double.

Ok, I took a more thorough look for any places that assume that
the return type is void or i64, and I was only able to find this one.

Oct 9 2014, 1:44 AM

Oct 8 2014

kmod retitled D5696: Add llvm.experimental.patchpoint.double from to Add llvm.experimental.patchpoint.double.
Oct 8 2014, 9:40 PM
kmod updated the diff for D5634: Enable invoking the patchpoint intrinsic.

Forgot to include changes to Intrinsics.td specifying that patchpoints can now throw

Oct 8 2014, 3:21 AM

Oct 7 2014

kmod retitled D5657: Reduce code duplication between patchpoint and non-patchpoint lowering. NFC. from to Reduce code duplication between patchpoint and non-patchpoint lowering. NFC..
Oct 7 2014, 5:51 PM
kmod updated the diff for D5634: Enable invoking the patchpoint intrinsic.

Remove the refactoring changes from this CR to move to a separate one

Oct 7 2014, 5:49 PM

Oct 6 2014

kmod abandoned D5572: Support invoking the patchpoint intrinsic.

I switched from the web interface to arc, and did not succeed in getting it to update the existing revision; closing this one in favor of the new one http://reviews.llvm.org/D5634

Oct 6 2014, 5:26 PM
kmod retitled D5634: Enable invoking the patchpoint intrinsic from to Enable invoking the patchpoint intrinsic.
Oct 6 2014, 5:20 PM

Oct 1 2014

kmod retitled D5572: Support invoking the patchpoint intrinsic from to Support invoking the patchpoint intrinsic.
Oct 1 2014, 7:13 PM

Jun 13 2014

kmod abandoned D3144: Update TailCallElim to avoid making redundant changes.

Looks like this feature got included in some recent broader work on TailCallElim.

Jun 13 2014, 2:46 PM