[PATCH] D34781: Introduce a MCReloc class

[Gerolf Hoflehner via llvm-commits]

> On Jul 19, 2017, at 9:26 PM, Rafael Avila de Espindola <rafael.espindola at gmail.com> wrote:
> 
> Gerolf Hoflehner via Phabricator <reviews at reviews.llvm.org> writes:
> 
>> Gerolf added a comment.
>> 
>> It is not clear to me if and how the original questions have been answered by this patch yet. Could you elaborate and add comments, please? Much appreciate!
>> 
>> What relocation should we map FK_Data_1 to? What if IsPCRel was set to true?
>> What is the addend? FixedValue or Target.getConstant()?
There is only one less use of Target.getConstant() (an assigned to Value = Target.getConstant() is only in the old code). I had expected more differences and also explaining more comments about FixedValue vs Target.getConstant(). Or maybe it wasn’t a big deal - then why mention it for justifying the effort?
>> What is the meaning of "foo - bar at got"?
>> There is *a lot* of duplicated code is the various obj writers.
The code size looks very similar before and after the patch. Maybe the follow up patch is supposed to show the merits? When I read “a lot” I thought a number of large almost equivalent code sniped in then-else-cases would disappear.
> 
> I am not sure I understand your comment. This is what this revision
> starts to fix.

I simply sense a gap between the goals you set out and the actual code. Perhaps I just need to see more of your patches to better appreciate what you are after.
> 
> For example, there is now only one addend: Reloc.getConstant(). It is
> also impossible now to represent "foo - bar at got" since we store a symbol
> instead of a symbolref for b.
Right, I see. I thought that ( foo -bar at got) had caused a bug, but I only found two places in the old code that had explicit checks to prevent it. So your patch rendered the checks redundant thankfully.
> 
> Cheers,
> Rafael