- User Since
- Oct 24 2016, 8:15 AM (203 w, 6 d)
Tue, Sep 15
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.
Tue, Sep 1
One minor nit, but otherwise LGTM.
Mon, Aug 31
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.
Fri, Aug 28
One minor nit, but otherwise LGTM. If Nemanja would still like the comment he suggested make sure to address that before committing.
Thu, Aug 27
Mon, Aug 24
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
Do you already have a test which dumps the relocations and verifies the addend on the relocations? (Do the llvm tools support dumping these new relocations?)
May 29 2020
May 26 2020
One comment but otherwise LGTM.
May 25 2020
May 21 2020
Thanks for implementing this MaskRay. I've left a couple comments, but still need to go through the testing.
May 15 2020
May 14 2020
May 11 2020
rebased to pick up NFC commit.
- Removed extra whitespace.
- Removed redundant testing (of fprs saves)
May 8 2020
May 7 2020
May 6 2020
👍 Thanks for trying this out Digger, I think this approach is the right one to take. I've had an initial look and left a few comments inline. I'll need to spend some time understanding ExternalSymbolSDNodes a bit better before being able to review the patch thoroughly enough to approve.
May 4 2020
Updated aix-calleesaved.ll test.
May 1 2020
Apr 30 2020
One minor comment but otherwise LGTM. Thanks for fixing this.
Apr 29 2020
Have you considered not transforming ExternalSymbolSDNodes, but instead supporting them for AIX and then creating the symbol and emitting the linkage for the ExternalSymbol machine operand in the ASMPrinter? The approach this patch takes adds complexity to a target independent part of the MC infrastructure. The amount of complexity it adds is not unreasonable, but I think if we can contain the complexity to the PowerPC backend its worth considering strongly even if the implementation there is more difficult.
I had to pull this as it breaks the PPC mutlistage LLD bot. Consider the following input: