- User Since
- Sep 21 2015, 12:36 AM (144 w, 5 h)
Together with D48472 that probably looks reasonable for me, thanks!
Fri, Jun 22
This patch seems to do a lot of unnecessary changes to the test cases.
Generally, a best practice for the patch is to do a minimal amount of changes,
though here I see unnecessary renaming changes, refactoring(?) changes etc in tests.
Thu, Jun 21
Tue, Jun 19
Mon, Jun 18
This is probably OK. LGTM with a nit.
I think this needs a test.
Fri, Jun 8
I think that:
- Simon is obviously an expert in MIPS. We have probably no other good reviewers on this platform here anyways.
- Patch has no critical stylish issues. I added few minor nits though. Anything else can be fixed after.
- It already took really too long to progress.
- Patch mostly affects MIPS parts of the LLD code (so it is isolated).
- Post-commit reviews are OK in general practice.
- It is important and desired patch for FreeBSD.
Mon, Jun 4
Fri, Jun 1
Thu, May 31
This is LGTM as is, but also added a suggestion about possible simplification.
I have no strong preference here though. I assume no much people will use this option anyways.
Wed, May 30
- Addressed grammar nits.
Tue, May 29
x86_64 ABI uses GOTPC32/GOTPC64 relocations for _GLOBAL_OFFSET_TABLE_:
@MaskRay, do you have a sample app that does not work for you and you are trying to fix? I started to wonder what exactly the real-life use case that needs to be fixed here :)
Particularly how _GLOBAL_OFFSET_TABLE_ is used.
Mon, May 28
One of the way I see how to solve it was suggested/described here: https://bugs.llvm.org/show_bug.cgi?id=36555#c19
Ok. After additional investigation (you mentioned TLS and I found a good sample for test),
I think we do the correct thing now.
New naming is confusing IMO. We had R_GOTONLY_PC_FROM_END, R_GOT_FROM_END and R_GOTREL_FROM_END.
Since names contains "GOT" it is clear that "FROM_END" says about the end of GOT.
I think this should be splitted into 2 patches: one for codegen and one for llvm-readobj tool.
May 26 2018
May 25 2018
I suggest separating R_X86_64_GOTOFF64 change from this patch to different one. I think it is completely independent.
May 24 2018
- Generate unique ID basing on begin symbol.
- Updated test case.
May 23 2018
What I can probably do is to compute ID based on the .text section name.
- Updated patch to match the behavior I suggested in latest comments.
(unconditionally add unique attribute)
I think that is fine. Updated the test cases as suggested.
- Updated test case (added no -ffunction-sections case).
- Do not do unification when no -ffunction-section is specified.
The problem of this patch is benchmark results I had.
May 21 2018
I'll update the status of this later.
May 18 2018
Was committed as r332688
May 17 2018
It looks a bit hacky. But I think it should work. Rui, what do you think?
! In D46874#1102734, @grimar wrote:
And renaming to something like .stack_sizes.XXX would require linker side change to place them into the single output section I think,
as currently they are merged by name, just like other regular sections.
This surprises me. I thought that default grouping would match up to the first '.', like it does for e.g. .text or .data grouping (i.e. .text.foo and .text.bar end up in .text). Or are some sections special-cased for this?
May 16 2018
This is for use with D46874. (currently LLD just crashes)
I switched to IR test case (and also fixed another one that turned out to be failing).
May 15 2018
Can we land it stand alone? It still brings a noticeable speed-up even without D45548.