[PATCH] D11969: Avoid using of DataLayout::getPointerSize() without address space argument in DWARF info generation

[Mehdi Amini via llvm-commits]

> On Aug 13, 2015, at 6:26 PM, Mekhanoshin, Stanislav <Stanislav.Mekhanoshin at amd.com> wrote:
> 
> Yes, I agree. A better longer term solution will require to dig in into each use of it.
> At the very least I think we need to remove TargetMachine.getPointerSize() and AsmPrinter.getPointerSize() not to increase a number of such functions without an argument.

If it is to replace this with a getPointerSize() that has no clear attached semantic, I don’t see this as a progress.

— 
Mehdi


> I'm testing this conservative patch now.
> 
> Stas
> 
> -----Original Message-----
> From: Mehdi Amini [mailto:mehdi.amini at apple.com] 
> Sent: Thursday, August 13, 2015 6:21 PM
> To: reviews+D11969+public+c218119e32184906 at reviews.llvm.org
> Cc: Mekhanoshin, Stanislav <Stanislav.Mekhanoshin at amd.com>; Arsenault, Matthew <Matthew.Arsenault at amd.com>; echristo at gmail.com; llvm-commits at lists.llvm.org
> Subject: Re: [PATCH] D11969: Avoid using of DataLayout::getPointerSize() without address space argument in DWARF info generation
> 
> 
>> On Aug 13, 2015, at 5:35 PM, Stanislav Mekhanoshin <stanislav.mekhanoshin at amd.com> wrote:
>> 
>> rampitec added a comment.
>> 
>>> In http://reviews.llvm.org/D11969#223472, @joker.eph wrote:
>> 
>>> 
>> 
>>>> - The DataLayout does not defined anything about which address space the code resides in
>> 
>>> 
>> 
>> 
>> I have checked with our engineer doing debug info, and it he confirmed these uses are not related to code addresses,
> 
> Not nice, my local debug expert had a different interpretation :(
> 
> It seems that every single use of this getPointerSize() will need to be studied to see what it really means and what is the “right” answer.
> 
> Right now MCDwarf.cpp is the only user of MCAsmInfo::getPointerSize() as far I can see.
> 
>> but generally to variable addresses. In fact in our case all code addresses are redirected, so we should not even hit it. Then a bigger type holding pointers is an inefficiency, but not a bug.
>> 
>> I.e. it looks like the name "CodePointerSize" is not correct, a "PointerSize" makes more sense. Maybe a "getLargestPointerSize()" and a "LargestPointerSize" would be better here.
>> It can even be calculated from the DataLayout itself.
> 
> It is not clear to me that it is the *right* answer to provide a “getLargestPointerSize()”, even though it seems that would be enough for your use case
> 
> — 
> Mehdi
>