Recent commit history on llvm: https://github.com/llvm-mirror/llvm/commits/master?author=annamthomas
- User Since
- Mar 30 2016, 11:13 AM (209 w, 6 d)
Sun, Apr 5
Thu, Apr 2
will update it.
p.s. Sorry for missing the functional issue the first time. All of the test changes should have made the issue obvious, but despite reading the LangRef description of signext, I somehow managed to miss the separation between ABI and optimization attributes.
thanks for the review Philip and pointing out the problem. All of us had missed the functional issue the first time around.
Wed, Apr 1
fixed missing code left out during rebase.
fixed buildbot failure. see above comments and added testcase test8.
whitelist valid return attributes and only add those. Added testcase for signext.
Tue, Mar 31
see above comment.
I got a failure in one of the binaryFormats:
Attributes 'zeroext and signext' are incompatible! %rev.i.i.i.i.i.i.i.i = tail call signext zeroext i16 @llvm.bswap.i16(i16 %ret.0.copyload.i.i.i.i) #6 in function _ZN4llvm7msgpack6Reader7readIntIsEENS_8ExpectedIbEERNS0_6ObjectE fatal error: error in backend: Broken function found, compilation aborted!
Mon, Mar 30
addressed review comments. Added two test cases: deref value being different, inlined callee body better optimized compared to callee.
Thu, Mar 26
addressed review comments
NFC w.r.t prev diff. Use VMap.lookup instead of a lambda which does the same.
Wed, Mar 25
Just chiming in here. We are planning to use addrspacecast for pointer width conversion (32 to 64 bit and vice-versa). Does not change the memory in anyway, i.e. if we lowered the addrspacecast into an IR call, that call could be marked argmemonly readnone. However, it is *not* a simple sign-extend/truncate either.
I've added some comments inline. Also, I think we have the scope to improve optimizations for addrspacecast at IR level, which is currently limited because different targets have varying lowering for addrspacecast.
Mon, Mar 23
Sun, Mar 22
Noticed while adding couple more tests, there were 2 bugs:
Fri, Mar 20
use isGuaranteedToTransferExecutionToSuccessor instead of MayThrow
Thu, Mar 19
refactored code, made asserts cleaner.
fixed clang tests. rot-intrinsics.c testcase has 5 different RUNs with 3 prefixes. Depending on target-triple, the attribute is added to the caller, so I've disabled the optimization for that specific test with -update-return-attrs=false
Will avoid confusing usage with cleaner asserts.
addressed review comments. fixed CI failure related to clang-tidy.
ah yes, I'd build llvm within it's own subdir without enabling any other projects. Build worked. thanks.
Tue, Mar 17
the test failures are related to attributes now being added to the calls in clang tests. All of these tests are using llvm intrinsics. Should we just disable this for intrinsic calls for now? Not sure how to update clang tests in the same patch as llvm project. Should be doable though.
addressed review comments. The analysis is conservative and test cases added show the negative cases where we should not backward propagate the attribute to the call (i.e. return value operand).
fixed clang format. NFC w.r.t. prev diff.
Mon, Mar 16
will update patch and place for review.
talked with Philip offline. We just need the check for non-trapping instructions between the return value (i.e. the call) and the return instruction. In other cases, it is either correct to propagate the nonnull-ness or it showcased a UB that already existed in the program.
Fri, Mar 13
hmm. yes, I can see this getting complicated for design and correctness. Thanks for the example @jdoerfert.
Just noticed this. I think we can special case this to: the return and def call are in the same basic block (it will take care of the if-else in the example) and has only one use which is the return. The latter takes care of avoiding incorrect semantics in that call do_sth_with_ptr_and_may_use_nonnull or any other use that depends on knowing which of the ptr is non-null.
addressed review comments. This handles all attributes on the call and only done after the cloning of the inlined body.
Tests added for attributes and making sure the original callee does not have the attributes added within its body.
Mar 6 2020
removed extra line. cleaned up whiitespace. NFC w.r.t. prev diff.
changed to early-return, updated comments and test.
Mar 5 2020
fixed dominator error.
abandon revision - too old and context lost.
abandoning this change - lost context.
Dec 4 2019
hi, thanks guys. Sorry for the abnormally late response (was on mat leave). Yes, looks like this can be abandoned.
Jan 29 2019
Jan 24 2019
The scalar indices don't appear to be a problem on a scalar gep, we even had a test for that.
just to note it is not a scalar gep - this is a vector gep which has a scalar index.
LGTM w/ comments.
Jan 22 2019
need to update comments and address review comments. moving out of queue for now.
Jan 18 2019
Jan 8 2019
thanks for reviewing the change Brian! Will land it soon.
This has passed all internal fuzzer runs - the DT verification failure tests have been added as test cases here after reduction.
we need to update domInfo for exits and those reachable from the exits.
Jan 7 2019
the same bug can happen with:
- latch exit
- non-immediate successors of latchexit/otherexit.
Jan 4 2019
The IDom can be any block within the loop (as mentioned in the comment).
So, we need to check for IDom *contained* in the loop.
Added test case shows where the IDom is within the inner loop of the original loop.
Both test cases pass now with the fix.
Jan 3 2019
@kuhar, I have not looked at the DTU uttility yet, but I'd think it can be rewritten (unless there are some limitations for the DomTreeUpdater utility). However, that would be a large enough change and I think it's better to do it as a separate change at a later point rather than as part of fixing this bug. Also, there are enough DT updates through out this code that it will take a while to get through the whole process of porting over for runtime unrolling.
Context and history for runtime unrolling: https://reviews.llvm.org/D35304
Dec 21 2018
Dec 5 2018
LGTM. Thanks for working through the revisions.
Dec 4 2018
Pls add -verify-memoryssa to the RUN command as well, now that MSSA is getting updated.
Nov 26 2018
Nov 22 2018
There's an LCSSA not being preserved bug in the original landed change (@dmgreen added the test case). As part of fixing it, pls add the following options to your RUN command in the test "-verify-loop-info -verify-dom-info -verify-loop-lcssa". This will make sure all forms of verification are checked.
Nov 20 2018
Nov 19 2018
Nov 16 2018
LGTM. Thanks for working through the comments.
Nov 15 2018
addressed review comments
based on above comments.
Nov 14 2018
Forgot to mark request changes. See previous inline comments.