- User Since
- Aug 18 2016, 4:39 AM (343 w, 4 d)
LGTM with the TODO in the test removed, thanks!
Sat, Mar 18
Fri, Mar 17
Just a heads up with are seeing a 10% regression caused by this change in a very SLP sensitive workload (the original source for the slp-fma-loss.ll tests). I still have to double check where the slowdown is coming from exactly.
LGTM, thanks for fixing this!
@cmatthews @azhar do you know if there are any tests for this part of the code?
Looks like multiple existing tests are triggering the assert and are failing.
Thu, Mar 16
Fix LGTM with comments by other people addressed.
LGTM, thanks for splitting this off!
The changes to make the tests more robust should probably be submitted separately so the diff only shows the functional test changes. Also, would be good to pre-commit new tests like vector_add_trip1024
Wed, Mar 15
Please make sure to update the commit message; it looks like the latest version doesn't address the poison issue IIUC
Sat, Mar 11
Fri, Mar 10
Address comments, remove ifdef code for ppc and adjust shadow-memory used on ARM64 macOS.
Update to use LLVMContext::MD_nosanitize , add nosanitize.ll test.
Bail out if global initializer cannot be evaluated (test added in a011823bff4d5399).
Thanks for the fix @vitalybuka ! I am surprised this wasn't covered by the added tests. Any idea why that is?
Thu, Mar 9
LGTM. thanks! But please include the full test (from D145676) in the commit with the fix, as the tests will otherwise fail on the UBSan bots.
Wed, Mar 8
Only call ConstantFoldLoadFromConst when new entry has been inserted.
Address latest comments & rebase before landing, thanks!
Rebase before landing, thanks for the review!
Tue, Mar 7
It looks like the test failures were from earlier runs, possibly with a missing dependency. I re-run them and they came back clear (on Windows, ClangdTests.exe/IncludeCleaner/GenerateMissingHeaderDiags failed, but that's very likely unrelated)
LGTM, thanks! The range is guaranteed to not include undef as per an earlier check.
LGTM with the suggestions addressed before landing.
LGTM, thanks! The commit title could be a bit more descriptive by explicitly mentioning what the code change is, e.g. something like: [LV] Use SetVector to collect invariants in calculateRegisterUsage.
It would probably be even better to make the threshold an argument (perhaps pass the threshold optionally like --filter-short=N), with the default being 1.0?
Mon, Mar 6
What are the current thoughts on the next steps to move forward with this. As for the 2 regressions, I think the one in llvm/test/Analysis/ScalarEvolution/incorrect-exit-count.ll won't be easily resolve-able. The other one can be solved by a follow-up (D144753), but it needs a bit more work because currently it triggers an infinite loop in a test case.
LGTM, thanks for updating this!
rebased and removed MIPS and PPC from compiler-rt/lib/tysan/tysan_platform.h as it has not been tested.
Address outstanding comments, remove some leftover commented-out code.
Use existing offset directly.
Rebase on top of 0ecef88.
Update to use ConstantFoldLoadFromConst to check the initializer value in the specifi field.
Not sure if replace_undef_X is an ideal name. Might be better to use something short like cX or condX