- User Since
- Feb 27 2013, 2:34 PM (329 w, 6 d)
I haven't had a chance to audit the whole patch yet, but in general the error suppression idioms are unsafe (though maybe no more so than the existing code?).
Sat, Jun 15
This is an excellent start!
Fri, Jun 14
Thu, Jun 13
This looks good to me. Thanks Praveen!
Fri, Jun 7
Mon, Jun 3
Thu, May 30
Tue, May 28
Mon, May 27
May 22 2019
May 21 2019
Gentle ping. Work on JITLink bought me some time, but I'm starting to hit situations where I would like to use this again. ;)
We don't want to clear unconditionally: we should (at least optionally) preserve the state so that clients can attempt recovery from transient errors (e.g. network drop-outs).
May 20 2019
Oh -- I missed that this was passing the name as a std::string by value. This will result in unnecessary string copies. In general for a case like this you would use an llvm::StringRef -- It's a lightweight string reference type. I'll switch to that before committing.
Looks good to me.
LGTM! Thanks Nick!
Yeah -- this seems to be missing the header side of the patch. You should be able to use the "update diff" option to create a new diff (still attached to this revision) with both sides of the change.
Committed in r361195.
May 17 2019
May 16 2019
Sorry I missed them, and thanks very much for the fix Sven!
May 15 2019
May 14 2019
Formally accepting so Phab does not gripe at me when this gets committed. :)
Looks good to me. Thanks so much for all your work on this Machiel!
Is it possible to write a test case for this? Otherwise LGTM. Thanks for all the work on this! :)
May 13 2019
May 12 2019
May 10 2019
May 9 2019
May 8 2019
I suggest we close this: The code is stale now, though the idea could be applied (and would be a neater fit) in the new LLJITBuilder utility.
May 7 2019
Great! Thanks so much Machiel!
Yes I agree it would be better if the Size member of MemoryBlock just held the allocated size. This would mean that r357058 and r351916 (and perhaps others) should be reverted.
See discussion on https://reviews.llvm.org/D61599.
Bah -- I missed this at the time. This should be reverted. We can rename methods to clarify, but MemoryBlock should hold the allocated size, not the requested size.
I don't think this is the right solution. MemoryBlock's purpose is to track the allocated memory. What the client does with that memory (including subdividing it) is their business. The client already knows the number of bytes they're requesting, so the fix for fragmentation in SectionMemoryManager should be limited to fixes in SectionMemoryManager.