[PATCH] D29170: [ELF] - Remove unnessesary checks from GC logic.

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 27 23:57:37 PST 2017


grimar added a comment.

In https://reviews.llvm.org/D29170#659009, @ruiu wrote:

> > Interesting. Is not it the same what you mentioned during review of https://reviews.llvm.org/D28612 ?
> > 
> >   "So, the right way of doing it is to set Live bit in the ctor if it should never be GC'ed."
> > 
> > 
> > Instead of checking the sections flags everywhere patch relies on Live bit now.
>
> Well, it is not a good idea to follow someone's advice blindly. You wrote this, so what do you think of this patch? I think that is important. I think this is not something I expected, and I believe there's a better way, but that's not this.


I did not follow your advice blindly, I tried that and found that it really reduces amount of changes I need to do for gc-support for emit relocs to single place.
Only because of that and after that I wrote that "it looks a good idea" and honestly I think this patch is very good, at least it IMO definetely makes code much better than it was before.
If it is not what you expected, then I looks I understood you wrong, but it does not change the situation and my opinion about that patch. May be your way you have in mind is better,
I do not know yet as I do not know what you expected :)

What is wrong with this patch ?

Rafael suggested to split https://reviews.llvm.org/D28612 to not support --gc-sections from start. I'll start from that then, though for GC case I believe we need some solution still, this or some another patch.


https://reviews.llvm.org/D29170





More information about the llvm-commits mailing list