- User Since
- Oct 24 2016, 8:15 AM (208 w, 18 h)
Fri, Oct 16
Tue, Oct 13
Fri, Oct 9
Thu, Oct 8
Thanks for splitting this out XiangLing.
Tue, Oct 6
Mon, Oct 5
Thu, Oct 1
More test coverage for this patch:
Wed, Sep 30
Tue, Sep 29
We are in a poor position since this depends on D75059 which I never commit-ed since as I realized it was not really NFC because we were wrongly inserting VR_SAVE code for AIX. This had the effect of emitting a fatal error if we used vector arguments, return values and I believe if we selected any vector instructions. I never went back and updated it because there was always something higher priority to work on. I have a non-NFC update I haven't posted which relies on the calling convention lowering to emit an error when passing vector arguments, and I add an error when returning vector arguments. I will post that shortly.
Mon, Sep 28
Fri, Sep 25
- Add static on helper functions.
- Use ## for comments in lit test.
- Remove redundant parens
- Ignore locals when looking for overriding symbol def.
Thu, Sep 24
2 minor comments but otherwise LGTM.
Wed, Sep 23
Address review comments, switched test to use split-file instead of other inputs and macros, and simplified the test as much as possible.
Mon, Sep 21
Does PPCMIPeephole::combineSEXTAndSHL have a similar problem that needs to be addressed?
Sep 15 2020
Extended the patch so that lazy object files are handled the same as archives, and bitcode files are handled same as native objects, both in archives and as lazy object files.
Sep 1 2020
One minor nit, but otherwise LGTM.
Aug 31 2020
Previous patch was not really NFC, since we were inserting VRSAVE pseudos on AIX, then emiting a fatal error in frame lowering. This was because we missed changing the condition gating the vrsave insertion to include the AIX abi.
Aug 28 2020
One minor nit, but otherwise LGTM. If Nemanja would still like the comment he suggested make sure to address that before committing.
Aug 27 2020
Aug 24 2020
I've spent some time reading through the linked thread. Thanks. There is a lot to reply to so I hope I don't miss anything.
- Removed now unused local variable.
- Removed -framepointer=all option in lit test.
- Fixed formatting of runsteps in new lit test.
Aug 18 2020
Aug 16 2020
Aug 13 2020
I believe we can have the optimization relocation on a code sequence like:
paddi ra, sym@pcrel load/store rt, offset(ra)
If so we should have a test showing we relocate it properly even if we don't optimize it yet.
Aug 12 2020
Aug 7 2020
Aug 6 2020
Aug 4 2020
Jul 29 2020
Jul 27 2020
Jul 24 2020
Jul 20 2020
Hey Digger, sorry for letting this sit for so long. Feel free to ping reviews periodically to get peoples attention back on them.
Jul 13 2020
Jul 9 2020
One comment but otherwise LGTM.
Jul 8 2020
I think we are really close now. I would suggest having a second lit test, which we link into either a static exec or pie exec which has calls using the R_PPC64_REL24_NOTOC relocation to symbols with global linkage and default visibility. I think it can be done with a single file (ie define all the callers and callees in the same file, no need for a second input file).
Jul 3 2020
Jul 2 2020
Patch looks very good Victor. I've left a couple minor comments in the implementation. I have a few suggestions in relation to the testing.
Jun 30 2020
LGTM. Thank Stefan.
Jun 29 2020
Jun 25 2020
Jun 24 2020
Looks good Zarko. I think we should either flip this over to show the stack growing down, or keep it as is but add High Memory at the bottom and Low memory at the top to show the stack grows downwards.
Jun 23 2020
Abandoning based on the discussion in D81457.
Jun 19 2020
Do you want to add a test for overflow of a 34-bit value?
Jun 18 2020
Jun 17 2020
Jun 16 2020
Jun 15 2020
Jun 11 2020
The CHECK-O section prints out the relocation and then it has symbol+<addend>. Is that what you are looking for?
Yeah, not sure how I missed that the first time. Sorry.
Jun 5 2020
Minor nit not related to this patch is that the test is really noisy. I realize this is the assembly clang emits, but a lit test can be cleaned up to just the needed constructs. Removing the comments, zero padding after the functions, local aliases (.Lexternal$local) and finally the size directives and the labels that are only needed for those directives goes along way in creating a much simpler test that helps to highlight exactly what functionality is being exercised at a glance. After landing this patch can you do an NFC cleanup of the test?
Jun 3 2020