User Details
- User Since
- Apr 11 2019, 6:15 AM (93 w, 21 m)
Fri, Jan 15
Committed as rG3676ef1053.
Thu, Jan 14
Ping.
Thu, Jan 7
This is causing compiler crashes during the second stage of the 2-stage buildbots (e.g. http://lab.llvm.org:8011/#/builders/7/builds/1140), so I've reverted it in rG76f6b125cef1.
Wed, Jan 6
This is causing a build failure on the 32-bit ARM bots:
Committed as 224328460d75e1fa16a23901b3aa494b89e0a842.
Tue, Jan 5
Dec 18 2020
Ok, LGTM
LGTM
LGTM
LGTM
LGTM
LGTM
LGTM
LGTM
Dec 17 2020
LGTM
LGTM
Why is this restricted to v7-A or later? The DSB and ISB instructions have existed since v6T2 and v6M.
LGTM
Dec 8 2020
The code changes look good, but this is missing tests for the new TLBI operands..
LGTM
Dec 7 2020
Dec 3 2020
Nov 19 2020
LGTM with one minor comment.
LGTM
The pre-commit test failures look relevant, so should be investigated.
LGTM
LGTM
LGTM
Also, __fp16 is a storage format and promoted to 'float' for argument passing
LGTM
Nov 18 2020
They are enabled iff the compiler supports _Float16.
Nov 6 2020
LGTM
LGTM
Oct 27 2020
I'm not aware of any official policy about this, but I don't think it's worth going through a multi-release deprecation process for something which is called out in the architecture manual as not valid.
Using x31 is confusing and not compatible with the GNU assembler, which are both god reasons to reject it. Also, the only occurrence of x31 in the architecture manual is this:
Oct 12 2020
LGTM
Oct 8 2020
Oct 2 2020
LGTM, but please split the unrelated change out into a separate patch.
Sep 15 2020
Sep 8 2020
LGTM
I don't know too much about statepoints or GC, but you have addressed all of @reames comments, and the AArch64 parts and code style look correct to me, so LGTM.
Sep 7 2020
Aug 25 2020
LGTM
Aug 7 2020
Aug 3 2020
This test case has been failing on many of the ARM and AArch64 bots all weekend, e.g. http://lab.llvm.org:8011/builders/clang-cmake-armv7-quick/builds/19408, would you mind looking into this?
Jul 21 2020
LGTM
Jul 17 2020
You haven't addressed my earlier inline comments.
Jul 16 2020
LGTM
Jul 14 2020
LGTM
Jul 9 2020
This was blocked on D76291 for a while, but that is now committed, so I'll re-land this today.
Jul 8 2020
Jul 6 2020
Jul 2 2020
Jun 24 2020
That sounds very fragile - I can easily imagine future, unrelated changes removing that COPY, or emitting one in the case which doesn't currently have one, which would completely change the effect of this patch.
Jun 18 2020
only if the register holding function's address got spilled
Jun 17 2020
LGTM
I don't think it makes sense to make f16 legal for targets which don't have any arithmetic operations on it, since that would be contrary to the definition of "legal". I'd also expect doing so to introduce a lot more complexity than this patch.
Jun 16 2020
I'd expect to see some extra checks to decide if reverting the indirect-call optimisation is profitable or not. If the loaded address is used more than three times, without being spilled, then I think it's better to leave the calls as indirect ones. If you don't think this is ever likely to be worthwhile for thumb 1, then it would be better to modify ARMTargetLowering::LowerCall to not do the transformation in the first place.
LGTM
Jun 15 2020
LGTM
Jun 12 2020
LGTM
Jun 10 2020
I think we could make this conditional based on whether the caller has BTI enabled, because:
- If the caller has BTI enabled, then we correctly use X16 for a BTI callee, or wastefully (but still with correct behaviour) use X16 for a non-BTI callee.
- If the caller has BTI disabled, then it must be in a page with BTI disabled, and so is able to use BR with any register, even if the destination is BTI-protected.
LGTM, with some comments which can be addressed later given the time-sensitivity of this.
LGTM. Maybe we should rename MQPR and DPR_VFP2 since they're not really specific to those architectures, but obviously not in this patch.
Is this expected to work for the soft-float calling convention, or is clang still passing half-precision values as integer types for that? If the former, then this needs some tests for that case.
LGTM, thanks for working on this patch series, this looks much clearer than doing it in clang.
LGTM
Jun 8 2020
I have the same concern as in D81402: will this still work if the call to the thunk goes through a long-branch veneer or PLT, which could clobber x16 and x17?
LGTM, inline comment could be done as a follow-up patch given that this is time-sensitive (recently published security vulnerability).
A few nits, but given the time sensitivity of this the only blocking one is how this will work with PLTs or long-branch veneers,
LGTM
The pre-commit test failures are in the newly added test, so need to be fixed before this can be committed. My inline comments could be addressed as follow up patches given that this is related to a recently-published security vulnerability.
LGTM
Since this patch adds loads, stores, moves and the calling convention, couldn't those be tested here, without waiting for intrinsics?