<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Thu, Aug 13, 2015 at 6:46 PM Mekhanoshin, Stanislav <<a href="mailto:Stanislav.Mekhanoshin@amd.com">Stanislav.Mekhanoshin@amd.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">> If it is to replace this with a getPointerSize() that has no clear attached semantic, I don’t see this as a progress.<br>
<br>
It looks like the question now is in the correct name of the function. It really looks suspicious to call it CodePointerSize in this case. Largest seems to be better because that is what actually happens there - pointer size is used in different contexts, but largest possible size works for all of them.<br>
<br></blockquote><div><br></div><div>The trick is that pointer size is going to be a dwarf type as pointer to <thing> and then used all over the place. It's a clever, but inaccurate workaround that "largest wins" here. It would stop you from correctly using something like say lldb to reconstruct function calls etc.</div><div><br></div><div>I think if you want to have disparate pointer sizes here you're going to need to change the DataLayout to encode a pointer size per address space or look at the places where we call getPointerSize() and actually somehow get the real size of the variable via something else. This might end up necessarily going back to DataLayout and a pointer size per address space.</div><div><br></div><div>-eric</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Stas<br>
<br>
-----Original Message-----<br>
From: Mehdi Amini [mailto:<a href="mailto:mehdi.amini@apple.com" target="_blank">mehdi.amini@apple.com</a>]<br>
Sent: Thursday, August 13, 2015 6:32 PM<br>
To: Mekhanoshin, Stanislav <<a href="mailto:Stanislav.Mekhanoshin@amd.com" target="_blank">Stanislav.Mekhanoshin@amd.com</a>><br>
Cc: <a href="mailto:reviews%2BD11969%2Bpublic%2Bc218119e32184906@reviews.llvm.org" target="_blank">reviews+D11969+public+c218119e32184906@reviews.llvm.org</a>; Arsenault, Matthew <<a href="mailto:Matthew.Arsenault@amd.com" target="_blank">Matthew.Arsenault@amd.com</a>>; <a href="mailto:echristo@gmail.com" target="_blank">echristo@gmail.com</a>; <a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
Subject: Re: [PATCH] D11969: Avoid using of DataLayout::getPointerSize() without address space argument in DWARF info generation<br>
<br>
<br>
> On Aug 13, 2015, at 6:26 PM, Mekhanoshin, Stanislav <<a href="mailto:Stanislav.Mekhanoshin@amd.com" target="_blank">Stanislav.Mekhanoshin@amd.com</a>> wrote:<br>
><br>
> Yes, I agree. A better longer term solution will require to dig in into each use of it.<br>
> 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.<br>
<br>
If it is to replace this with a getPointerSize() that has no clear attached semantic, I don’t see this as a progress.<br>
<br>
—<br>
Mehdi<br>
<br>
<br>
> I'm testing this conservative patch now.<br>
><br>
> Stas<br>
><br>
> -----Original Message-----<br>
> From: Mehdi Amini [mailto:<a href="mailto:mehdi.amini@apple.com" target="_blank">mehdi.amini@apple.com</a>]<br>
> Sent: Thursday, August 13, 2015 6:21 PM<br>
> To: <a href="mailto:reviews%2BD11969%2Bpublic%2Bc218119e32184906@reviews.llvm.org" target="_blank">reviews+D11969+public+c218119e32184906@reviews.llvm.org</a><br>
> Cc: Mekhanoshin, Stanislav <<a href="mailto:Stanislav.Mekhanoshin@amd.com" target="_blank">Stanislav.Mekhanoshin@amd.com</a>>; Arsenault, Matthew <<a href="mailto:Matthew.Arsenault@amd.com" target="_blank">Matthew.Arsenault@amd.com</a>>; <a href="mailto:echristo@gmail.com" target="_blank">echristo@gmail.com</a>; <a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
> Subject: Re: [PATCH] D11969: Avoid using of DataLayout::getPointerSize() without address space argument in DWARF info generation<br>
><br>
><br>
>> On Aug 13, 2015, at 5:35 PM, Stanislav Mekhanoshin <<a href="mailto:stanislav.mekhanoshin@amd.com" target="_blank">stanislav.mekhanoshin@amd.com</a>> wrote:<br>
>><br>
>> rampitec added a comment.<br>
>><br>
>>> In <a href="http://reviews.llvm.org/D11969#223472" rel="noreferrer" target="_blank">http://reviews.llvm.org/D11969#223472</a>, @joker.eph wrote:<br>
>><br>
>>><br>
>><br>
>>>> - The DataLayout does not defined anything about which address space the code resides in<br>
>><br>
>>><br>
>><br>
>><br>
>> I have checked with our engineer doing debug info, and it he confirmed these uses are not related to code addresses,<br>
><br>
> Not nice, my local debug expert had a different interpretation :(<br>
><br>
> 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.<br>
><br>
> Right now MCDwarf.cpp is the only user of MCAsmInfo::getPointerSize() as far I can see.<br>
><br>
>> 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.<br>
>><br>
>> 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.<br>
>> It can even be calculated from the DataLayout itself.<br>
><br>
> 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<br>
><br>
> —<br>
> Mehdi<br>
><br>
<br>
</blockquote></div></div>