- User Since
- Mar 24 2014, 8:27 AM (444 w, 1 d)
Feb 20 2018
Feb 12 2018
Dec 21 2017
Dec 19 2017
Dec 18 2017
Dec 15 2017
Removed the unnecessary isSized() check
Will replace with a series of individual review requests.
Dec 13 2017
could someone take a look at this or maybe suggest a different reviewer if I chose poorly, please?
Dec 1 2017
Nov 4 2017
Oct 5 2017
Updated to address comments by rnk.
Oct 4 2017
Apr 17 2017
Since I had a hand in changing the logic for the live interval calculation, I
feel like I should take some time to explain the approach that was taken here.
Apr 16 2017
Jan 31 2017
LGTM except for the style nitpick which is just personal preference. I don't think I'm allowed to actually accept patches though.
Mar 15 2016
Aug 21 2015
I don't have a plan for this yet. Basically, it seems like we'd have to check for each terminator that is not dominated by a lifetime end whether it is dominated by a lifetime start for each alloca in the function that is being inlined. I'm not sure whether that's feasible or if it would end up being too expensive. Any input is very welcome.
Aug 20 2015
I had to revert this because it caused test failures in clang, which made me realize that this interacts badly with inlining as it is. The inliner only adds lifetime intrinsics for an inlined alloca if there are no lifetime intrinsics for this alloca at all. But with this patch, we can end up with an alloca that only has a lifetime start but no end. In that case, the inliner wouldn't add lifetime intrinsics, leading to worse stack coloring than before.
Aug 18 2015
Updated to correctly handle lifetime intrinsics on undef pointers
Aug 14 2015
[DSE] Enable removal of lifetime intrinsics in terminating blocks
The test with invoke was just to have an unwind block, because that's what my original testcase had. I'll remove that test because with just a call it adds nothing that isn't already handled in the other tests.
Aug 11 2015
Replaced the original approach of ignoring lifetime intrinsic when looking for
empty landing pads with an implementation that enhances DSE to remove lifetime
intrinsics from terminating blocks.
Aug 10 2015
In my case, the callee also contained a call to llvm.lifetime.end, so I didn't consider the removal a problem even if the callee is inlined. But of course you're right, in the general case this could preclude further optimizations.
Aug 2 2015
May 25 2015
May 11 2015
Could somebody have another look at this?
Apr 30 2015
Updated the patch to use getConstantOnEdge to improve readability. Added a
simpler test case without a loop and a negative test case. LVI doesn't support
vector types yet, so there's no test for that, but I left a note that support
for vector types may be added later.
Apr 29 2015
FWIW, this helps with an optimization problem we hit with the rust compiler:
Apr 26 2015
Apr 17 2015
Updated the patch to only apply the optimization to selects with a scalar condition.
Apr 16 2015
Mar 4 2015
Mar 1 2015
Feb 10 2015
Not sure if this still qualifies for 3.6 (I guess it's not strictly a regression since llvm.assume is new?), but it'd be nice if it would as we hit this bug with rustc when using llvm.assume.
Feb 8 2015
Fixed the line length.
Feb 7 2015
Added a FIXME for MD_tbaa_struct and MD_mem_parallel_loop_access
Jan 28 2015
Updated the header in metadata.c to mention the second command implemented in that file. Sorry for the noise.
Added an MDNode extraction function with proper assertions, fixed
LLVMAddNamedMetadataOperand, and wrote a test for it.
Jan 27 2015
Thanks for the feedback!
I guess I should mention that triggering the assertion is a regression from 3.5.
Jan 25 2015
Jan 13 2015
Jan 2 2015
Oct 16 2014
Sep 9 2014
Committed as r217453
Sep 8 2014
I have commit access by now, can I just commit the test or was there a reason it was left out?
Sep 2 2014
Thanks, did not known about "not" before. Updated the test case to use it.
Added a test case.
Sep 1 2014
Had another look, still couldn't find anything. The code is either using GetElementPtrInst instead of GEPOperator, calls getType()->getPointerAddressSpace() or checks for vector types beforehand, so it doesn't trigger the bug. Alias analysis seemed like it could work, but I didn't manage to get a testcase with that either (and I'm too clueless about that code).
Aug 31 2014
Updated the patch to use getPointerAddressSpace(), couldn't find a different
pass that triggers the bug though, so the test still uses mergefunc
Aug 15 2014
No, I don't have commit access.
Updated the patch to introduce a helper to combine instruction metadata and
handle all metadata for the hoisted instructions.
Jul 30 2014
I should note that I do not have commit access and need somebody to commit this for me.
Jul 28 2014
Could somebody please review this patch? Thanks!
Jul 18 2014
Jun 25 2014
Updated the tests for invoke-with-ranges to avoid the bitcasts
Jun 22 2014
Jun 17 2014
Committed in r211082
Jun 16 2014
Fix SROA regression causing miscompilation, fixes PR19250
Jun 12 2014
yes, I need someone to commit this for me.
Jun 11 2014
Reverted back to the patch the avoids the merge in case of different range
metadata and adjusted the tests to check for the whole function body remaining
Jun 10 2014
Sorry, forgot to remove the old @test25 from the original patch.
Added a comment to explain why the whole if-else block has to be moved after
the early continue. Moved the original test into its own file and added new
tests for the slice order indepence of the chosen type for the alloca.
Updated the patch to merge the range metadata for the loads instead of not
merging such functions at all. I found mergeTwoFunctions() to be a better place
than insert() to call the new method that handles the merge of the range
metadata. I hope this is what you had in mind.
Jun 9 2014
I wasn't sure about expanding the range to be more generic, since I thought that that might defeat some optimizations in other passes. I'll update the patch later today.
Jun 8 2014
Jun 5 2014
Removed the now obsolete changed to mergefunc, keeping just the regression tests
Hi Sean, Hi Nick,
May 29 2014
I'd be grateful if somebody could take the time to review this. The regression fixed by this is blocking rust (the language) from using i1 for its bool type.
May 9 2014
Apr 23 2014
Really fix the -stats related part of the test
Fixed the test to include -stats
Fix a mergefunc crash caused by bitcasting intrinsics
Yeah, that's what I meant earlier. The result would look like this:
Apr 22 2014
I probably shouldn't have said crash. The mergefunc pass completes. Without merging the functions. But in enumerate, it created that bitcast of the intrinsic, which is added to the list of users of that intrinsic. What happens then is that the verify pass checks all users of all intrinsics, sees the bitcast, complains about it and aborts.
do you still want the test to be adjusted, given that the functions aren't actually merged? If so, I'd have to increase the size of both functions, for it to make any sense, because they're currently too small to be merged anyway. Doing so would be ok with me, but doesn't seem relevant to the bug at hand.
2014-04-23 7:52 GMT+02:00 Stepan Dyatkovskiy <firstname.lastname@example.org>:
I suspect this code:C1 == ConstantExpr::getBitCast(const_cast<Constant*>(C2), C1->getType());
We didn't expect it will return "true" for such bitcast (from "i8* @breaker(i8*)" to "i64 @llvm.bswap.i64(i64)". Those are different functions. Perhaps *this* is the real bug?