Recent commit history on llvm: https://github.com/llvm-mirror/llvm/commits/master?author=annamthomas
User Details
- User Since
- Mar 30 2016, 11:13 AM (327 w, 20 h)
Jun 1 2022
@nikic, this change caused major degradations to our downstream benchmarks. Basically, in the good case, we used to have selects (which got converted to BLENDVPD instruction). I wonder if there's a reverse transform somewhere when we have branches without profile and change them into selects.
Abandoned in favour of D126777.
This patch works for the crashes we saw downstream. Thanks!
May 31 2022
simplified test case
Could you pls elaborate? Unfortunately, I don't know what needs to be done here and I think irrespective of any vectorization "performance issue", we would need to fix the *functional problem*.
JFI: It also requires updating a lot of tests, around 25 in X86 with CHECK lines updated since the insertion point changed. I have not tested the other targets, but there would be others as well.
@ABataev, we are getting an assertion failure downstream in setInsertPointAfterBundle which is bisected to this patch and reproducible with it.
May 30 2022
went another path.
May 26 2022
Apr 29 2022
Apr 28 2022
addressed review comments.
Apr 27 2022
already landed. closing as done.
When tested on a large method from our internal workload, I see about 12% reduction in BranchFrequencyAnalysis (other analyses time also dropped to a lower degree). Overall opt time also dropped about 4%.
Apr 26 2022
thanks for the quick review!
Apr 21 2022
Apr 20 2022
Apr 13 2022
Apr 11 2022
Mar 11 2022
Feb 24 2022
Do we have any compile time measurements for this change with O3?
Feb 14 2022
LGTM with a comment. Great job on root causing and fixing this!
Feb 2 2022
Feb 1 2022
I spend sometime trying to see if the failing premerge tests seem related or maybe false positives, but I have no idea where these are from. Also, other completed unrelated review show the same failures: https://reviews.llvm.org/D94928
Jan 31 2022
support system and singlethread syncscopes. Added test for target-dependent syncscope to showcase need for isIdenticalTo check.
addressed review comments. Added extra tests.
fix based on Philip's comments about scopes.
Jan 28 2022
already handled as needed.
Jan 18 2022
Looks like the comments have been addressed regarding the split. Thank you! Moving myself out of blocking state.
Jan 11 2022
At this point, everything necessary is landed upstream through the different approach. See linked patches from Philip.
Jan 5 2022
There are 14 test failures in trunk, which disappear once this change is reverted. I wonder why the Buildbot didn't catch it.
Here's 13 of them with the .yaml extension:
Failed Tests (13): LLVM :: tools/llvm-dwarfdump/X86/verify_curanges_incomplete.yaml LLVM :: tools/llvm-dwarfdump/X86/verify_die_ranges.yaml LLVM :: tools/llvm-dwarfdump/X86/verify_invalid_cu_ref.yaml LLVM :: tools/llvm-dwarfdump/X86/verify_invalid_die_range.yaml LLVM :: tools/llvm-dwarfdump/X86/verify_invalid_ranges.yaml LLVM :: tools/llvm-dwarfdump/X86/verify_invalid_ref_addr.yaml LLVM :: tools/llvm-dwarfdump/X86/verify_invalid_ref_addr_between.yaml LLVM :: tools/llvm-dwarfdump/X86/verify_invalid_rnglists.yaml LLVM :: tools/llvm-dwarfdump/X86/verify_invalid_stmt_list.yaml LLVM :: tools/llvm-dwarfdump/X86/verify_invalid_strp.yaml LLVM :: tools/llvm-dwarfdump/X86/verify_lexical_block_ranges.yaml LLVM :: tools/llvm-dwarfdump/X86/verify_overlapping_function_ranges.yaml LLVM :: tools/llvm-dwarfdump/X86/verify_overlapping_lexical_block_ranges.yaml
Jan 4 2022
@spatel: inline comment added about incorrect arguments for one of the callsites. Pls fix.
Jan 2 2022
Dec 16 2021
Good catch!
LGTM
Dec 13 2021
I don't think we generally do this. I will need to add either the allocations or the paramcase soon-ish after this. But I see your point (since the change as-is does not affect any upstream code). Will wait if @reames has any concerns.
Dec 9 2021
Note that this is a rewrite of original patch to handle sinking of function call with outparam(s). I have also added allocation test cases to show that these are not affected by the patch.
rewrote for outparam usecase in simplest form
Change LGTM. @jdoerfert any additional comments?
LGTM.
Dec 8 2021
Nice find! This is obviously a good thing to do for unrolling. Any ideas why there's no vectorization test cases to update since we do support multi-exit vectorization?
Dec 7 2021
Dec 6 2021
Actually looking at the patch again, you're mixing two concepts here:
- fixing the LangRef and Attributes.cpp for align attribute on vector of pointers should be done as an independent patch. It is orthogonal to the SelectionDAGBuilder code and avoids the false dependency created by the parent patch linked to this patch.
- ABI intrinsics and SelectionDAG builder changes can depend on the parent patch.
Dec 3 2021
Thanks Philip. This is a definitely interesting example worth handling. Let me update the code to consider this use case and precommit some of the tests. The use case we have downstream (as mentioned is object.hashCode), but this example makes a good usecase for upstream IMO.
Dec 1 2021
updated to correct diff.
rebased
rebased
addressed review comments
Nov 26 2021
separated out API
Nov 23 2021
This change looks good to me and thank you for taking this through!
Oct 19 2021
addressed review comment about naming. Updated a comment in the testcase to parse better.
I think that's a good idea. Will do as a follow-up after landing patch.
Oct 15 2021
ping
minor changes in naming, clang format. NFC w.r.t. previous version.