- User Since
- May 29 2017, 8:02 AM (172 w, 6 d)
Fri, Sep 18
FYI: All of the dependent patches have landed so you should now be able to just apply this patch on top of trunk.
Updated the patch to use the NOP constant.
Updated test case to check for .got.
Tue, Sep 15
Mon, Sep 14
Replace error() with errorOrWarn().
Fixed formatting in source file.
Fixed spacing in test file.
Fri, Sep 11
I've updated the description to only use the correct version of the relocation: R_PPC64_GOT_TLSGD_PCREL34.
Fixed formatting and comments in a few places.
Added --soname to the test file.
Thank you for the reviews!
Tue, Sep 8
It seems that most builders do have the extra_configure_args parameter but the builder that we use here does not.
You may have to add the parameter to the build factory to get this to work.
This mostly LGTM.
From my perspective you can add no-show-raw-insn on commit.
Thu, Sep 3
Removed a couple of variables that could be inlined.
Added an error in case the offset from the 4 byte alignment is 2 or 3.
Moved checks in the test case.
Fixed a couple of things on the test case.
Added the function name inside <>:.
Removed the .size directives.
Added SYMBOL-NEXT: where possible.
Removed the linker script that was not needed.
Added a quick summary of the test case.
Fixed nit and moved the position of the case in the switch.
Tue, Sep 1
Mon, Aug 31
Fri, Aug 28
Wed, Aug 26
Mon, Aug 24
Fri, Aug 21
Aug 21 2020
Also, please add a test where the offset is not zero.
So the assembly would be for example:
paddi 3, 13, x@TPREL+8, 0
Adjusted one comment and moved the code around to merge the two errorOrWarn.
Aug 20 2020
Only minor nits form me.
Couple of nits.
Updated the comment to make it more concise.
Updated the test case to add the missing : .
Aug 19 2020
Fix some formatting.
Fix a couple of nits.
Add the --triple to the test case.
Aug 18 2020
The general idea makes sense to me.
I have a set of mostly minor comments here and there.
Renamed the assembly test file and removed the other test file.
Added the new assembly test.
Yes! That's the test I need.
Let me add that test and we can choose which one is preferred.
I've merged a couple of the tests using yaml2obj -D. I would prefer not to merge the tests any further because the actual code in the .text section is different between TLSGD and TLSLD.
Aug 17 2020
I've tried to address the concerns for this patch. I'm sorry that it took so long to make the code changes.
Aug 14 2020
Aug 12 2020
This patch makes sense to me.
I have a question about the access instruction but I don't think we should worry about it now.
Aug 10 2020
Aug 7 2020
Removed the attribute nounwind as it is not needed in the test case.
Fixed the mcpu=future to make it mcpu=pwr10.
Aug 6 2020
I think this looks good! Thank you for fixing this issue.
Aug 5 2020
Jul 28 2020
Overall I think this looks good. I had a handful of comments but nothing major.
I wish we had a better way of doing the conversion between the old forms and the PCRel forms but I can't think of a better way.
Jul 27 2020
Changed the way that the error is emitted.
Jul 23 2020
I only had one nit.
Jul 22 2020
Jul 21 2020
Jul 20 2020
Flipped the if statement.
Fixed a comment in the source and added a couple more comments in the test case.
Updated patch to correctly remove new fixup.
In order to create the fixup I can use:
MCFixup::create(Offset, Expr, FirstLiteralRelocationKind + R_PPC64_PCREL_OPT, Loc);
Removed the fixup name.
@MaskRay Let me know if this is what you were looking for.
Jul 17 2020
Please wait for Lei to give the approval as well before you commit.
I should not have changed the cast.
Changed it back now.
I only had one question.
Modified static_cast to cast.
Made use of MCSymbol::print.
Jul 16 2020
I think the tests can be cleaned up a little but otherwise this LGTM.
I only had a couple of comments.
Overall I think this looks good.
Forgot to rename the testcase in the previous update of the patch.
Renamed it now.
Just a few nits for this patch.
The title of the patch mentions both zero extend and sign extend.
However, it seems that we only have instructions for the zero extend case. Is that right?
I see both types of tests in:
But I only see codegen tests for the zreo extend version.
Changed the way that the peephole finds candidates according to the suggestion by Nemanja.
Added/fixed some comments.
Cleaned up the testcase.
Jul 15 2020
Moved the rest of the work to the target indep side after the patch D83751.
Jul 14 2020
Nope, no more comments.
Thank you for putting up a patch in response to your comment.
Once this goes in I'll rebase D79625 on top of it and that patch should become a lot smaller.
Jul 13 2020
Fixed a couple of spacing nits.