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 (251 w, 4 d)
Dec 8 2020
Great, thanks!
missed one review comment: move DL initialization to runImpl.
addressed review comments
Yes, pls rename the function since it actually represents which passes are not yet ported to NewPM.
Also, I have run the tests with enable-new-pm=true. However, there are 36 failures which are already failing with new-PM without my change (had to build both with and without my change to confirm this). Is there some buildbot that tracks the NewPM option? It will make it easier to catch if any testcases regress...
Dec 7 2020
ping.
Dec 6 2020
missed adding new header file to diff.
Dec 3 2020
rebased over landed NFCs.
Dec 2 2020
Nice catch! LGTM from me. One comment inline. Pls see if @fhahn has any comments as well?
addressed review comments
Dec 1 2020
missed adding new file into commit.
Nov 18 2020
Nov 17 2020
Nov 16 2020
Nov 12 2020
Just recording here (previous discussion was here D90742) :
our internal buildbots at Azul has failed because of an older assembler. We’ve taken in all commits/reverts/relands w.r.t this change upto Nov 4th. I have reverted the change rG8383fddc4fa9 downstream and confirmed the build passes.
We use the GNU assembler and it is configured for 64 bit machines. Here's the details from one of the machines:
as --version GNU assembler version 2.20.51.0.2-5.47.el6_9.1 20100205 Copyright 2009 Free Software Foundation, Inc. This program is free software; you may redistribute it under the terms of the GNU General Public License version 3 or later. This program has absolutely no warranty. This assembler was configured for a target of `x86_64-redhat-linux'
Also, note that I tagged this against the incorrect review. The change causing the failures is https://reviews.llvm.org/D90592. I'm going to paste this there and we can continue discussion there.
Nov 11 2020
Just a stylistic comment. You can have the InvariantCond as a struct with the fields: InvariantPred, InvariantLHS, InvariantRHS. I think this will make code more readable (for example, use the struct in ReplaceInvariantCond(ExitingBB, InvariantCond).
hi @courbet,
our internal buildbots at Azul has failed because of an older assembler. We’ve taken in all commits/reverts/relands w.r.t this change upto Nov 4th.
Nov 10 2020
Hi @nikic, some comments below.
Ah, I think your correctness fix is what Johannes was implying by the "multiple args captured" in a single call.
Nov 5 2020
Nov 3 2020
Hi @kazu, just wanted to point out - We have seen multiple assertion failures in benchmarks downstream with this patch (or a related one built on this patch). Trying to revert this downstream shows there are dependent patches. Is this assertion seen and already fixed in trunk?
may revisit later.
Oct 30 2020
Oct 14 2020
Oct 7 2020
addressed review comments, rebased and updated for AArch64 (this patch is NFC in behaviour for AArch64).
Oct 6 2020
Hi Artur, just to explicitly state out:
But element atomic memcpy/memmove intrinsic calls might be generated by the optimizer, which is not aware of this coupling. If statepoints without deopt into are not allowed and we see a non-leaf memcpy/memmove without deopt state we treat it as a leaf copy and don't produce a statepoint
If a long running loop of loads and stores are converted to atomic memcpy (through loop idiom recognize for example) and we don't produce a statepoint - this will make the memcpy non GC parseable, which is erroneous. So, it is the responsibility of such runtimes to also introduce some mechanism to prevent long running loops from being converted to memcpys (such as having safepoint requests within the loop thereby preventing long running loops from being converted to memcpy: https://llvm.org/docs/Statepoints.html#id27).
Sep 29 2020
addressed review comments. Bails out in case of overflow (added testcase for multiply overflow).
Sep 24 2020
addressed review comments about splitting out patches. This patch handles complex addressing mode.
Sep 18 2020
thanks for this Philip!
Sep 10 2020
addressed review comments. Added more testcases (positive and countercases).
Sep 8 2020
handle clang-tidy comment.
Sep 7 2020
handles complex addressing mode using base and index operands. All test cases pass.
I plan to add more counter examples once the basic idea is agreed upon.
Sep 4 2020
addressed review comments.
While working on a follow-up change, I realized we don't need to separate out whether we're handling simple or complex addressing mode based on whether we fault in zero page or not. Both are orthogonal issues. The reason I coupled them is because we still need a way to get the "total constant address" *if it exists* and make a distinction between when we succeeded to get one versus when there was no such constant address apart from the offset.
thanks Denis for your comments. I'll let others have a chance in review as well. Regarding broader audience today, implicit null checks works only with X86 (or rather that's what we test with). Also, none of the global APIs are changed, precisely because they are used in other parts of the code. Hence a new API is introduced and used currently only for implicit null checks. It is broad enough that we can reuse it in other passes if we wish to do so.
addressed review comments around using simpler form rather than isTied. Whitespace testcase changes avoided. NFC w.r.t. prev diff.
Sep 3 2020
Sep 2 2020
Jun 4 2020
Code LGTM. quick glance at tests.
Jun 2 2020
LGTM w/ comments inline.
May 26 2020
LGTM. thanks for working on this! Apart from dropping all of the extra handling code in RS4GC and verifier (which itself is a really good reason for this support btw), I'm assuming this for optimizations which know about the deopt_op bundle (or gc_transition bundle) after relocations are made explicit? i.e., a general robustness patch.
May 15 2020
thank you for the review!
May 14 2020
addressed review comment about test case.
addressed review comments. Added unittest case.
Landed as bb308b020522420413c7d3f2989a88f2fc423c56.
May 13 2020
Thanks Johannes. @fpetrogalli Francesco, I believe I've addressed your concern as well?
Hi @fpetrogalli,
Yes, but it should be orthogonal to that requirement. I think hiding the mangling behind a static function, while the demangling is exposed seems unintuitive. Any front-ends that need to attach the attribute will need this function exposed.