<div dir="ltr">Hi Rafael,<div class="gmail_extra"><br><br><div class="gmail_quote">2014-05-17 21:15 GMT+08:00 Rafael Espíndola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span>:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class="">> I don't think Nick is understanding this issue correctly as I mentioned in<br>

> my last reply, so I'm not sure if you are really agree with him. The issue<br>
> you mentioned below is actually completely different from Nick's point.<br>
> Refer to my answer below, anyway.<br>
<br>
</div>The part about unnamed_addr, yes, but not the part about alignment,<br>
right? Things like getGlobalMergeAlignment do look out of place.<br>
<div class=""><br>
>> Is<br>
>> this uncommon enough that checking for uses coming from the same<br>
>> function was not considered profitable?<br>
><br>
><br>
> I'm not sure if this could be profitable, because they are global variables<br>
> and we don't really know how they are being referenced in other modules. The<br>
> heuristic of considering the uses within a function for current module would<br>
> not probably give the same benefit to other modules, so I'm thinking the<br>
> heuristic strategy of merging them all together for current module would not<br>
> make big difference from the one of further checking uses within a function<br>
> for current module.<br>
><br>
> I never say my optimization patch is commonly profitable, and this is why<br>
> they are under switches control. But for some specific cases, it would be<br>
> profitable, I believe In particular, for the case that global variables are<br>
> being heavily used in the module that are defining them.<br>
<br>
</div>Well, we normally don't want to have too many use facing switches.<br>
What is the plan for removing this one? If variable concatenation (to<br>
avoid confusion with merging which causes the address to be the same)<br>
is profitable enough for ARM/AARCH64 that it is better than the<br>
inability to dead strip some code, then it should always be enabled.<br>
If not, we should figure out an heuristic for when to do it.<br></blockquote><div><br></div><div>I'm not sure if I'm understanding your point correctly. There might be several different functions in a module(file), and you are asking for concatenating symbols being used in a given function, so how do we handle multiple multiple functions? Can you clarify this? One solution I could have is we use a heuristic rules to concatenating symbols in a given function as possible as we could, but we still need to merge all symbols all together into a single MergedGlobal symbol. Or you mean you want to define multiple MergedGlobal symbols and map them to each function? If this is the case, how would you handle the conflict among different functions?</div>
<div><br></div><div>If essentially you are proposing to remove the unused symbol of current module in MergedGlobal variable. I think this is a good point to me, because anyway it's useless on helping optimization current module.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
It should also be done really late. In particular, it should probably<br>
not run with -flto and instead be enabled in the LTO pipeline.<br></blockquote><div><br></div><div>I don't understand your point here. What should be done late? Do you mean the GlobalMerge pass should be done late? At present, this pass happens in PreISel stage, so where do you want to put it?</div>
<div><br></div><div>This optimization itself doesn't rely on LTO at all. But at present ARM64 already has LTO solution using LOH, we are thinking it can be orthogonal with this static global merge solution. Yes, LTO is useful, but it can replace static solution, and a lot of end-users prefer to use static solution only. </div>
<div><br></div><div>Previously you also mentioned getGlobalMergeAlignment is out of place, so if you are talking about it early to to set alignment for GlobalMerge variabls, then could you please let me know where you think we should put it? I thought it was quite natural to set it at the place of generating this GlobalMerge symbol in global merge pass. </div>
<div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
I also don't follow your objection to looking at the function bodies<br>
to decide when it is profitable. If a variable is external, codegen<br>
must produce the fully general access pattern. That is, the only code<br>
that benefits from concatenating global variables is the one in the<br>
same TU as the variables, right?<br></blockquote><div><br></div><div>Oh, I think I didn't say I object to checking function bodies, and I only said it's also a kind of heuristic just like my simple solution of not checking function bodies. But I realized that excluding "dead symbol" is a good idea I should follow.</div>
<div><br></div><div>Sorry what does TU stand for?</div><div><br></div><div>The benefit for AArch64 is to reduce the ADRP (get page address of global symbol) and ADD instructions.</div><div>(1) If two symbols are merged in the same data structure, they can share the same base address.</div>
<div>(2) the offset of symbols to the page address is less than a page, offset can be encoded into instruction, and we will save ADD instruction, and this is why we want to set alignment in global merge pass.  </div><div>
<br></div><div>Hopefully I described the problem clearly.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">

<div class=""><br>
>> putting two variables that a function uses next to each other would<br>
>> also be a win there, no?<br>
><br>
><br>
> X86 and ARM/AArch64 have different address model, and this is because of the<br>
> so-called CSIC and RISC differences. X86 can directly encode global address<br>
> relocation into instructions, while ARM/AArch64 can only encode it in<br>
> load/store instructions. For AArch64, this is getting worse, because ADRP<br>
> instruction can only get the page address of global variable due to the<br>
> limitation of instruction encoding size (32-bit only), and we have to<br>
> introduce another add instruction to get the 64-bit address. This is why<br>
> this optimization is very meaningful to AArch64. I think you would be able<br>
> to understand this much better if you look into the very beginning part of<br>
> my patch review thread.<br>
><br>
> Yes, putting two global variables that a function next each other would be a<br>
> win, but as I mentioned above this is also heuristic because we don't know<br>
> how global variables are being used by other modules at compilation time.<br>
<br>
</div>It is heuristic. I was just pointing out that it is not fundamentally<br>
an ARM only thing.<br>
<div class=""><br>
><br>
> OK. Thanks for letting me know that. Hopefully my patch still can reuse your<br>
> new GlobalAlias solution. Looking at the bug tracker you pointed to me, it<br>
> seems to be a very old problem raised by Christ long time ago. I'm not sure<br>
> if you can really make it happen next week. I would be extremely<br>
> appreciative if you do.<br>
<br>
</div>I think I can. The main change is already in. What is missing is just<br>
the adding offsets.<br></blockquote><div><br></div><div>I noticed that you already sent out the patch of supporting alias offset, so appreciate your quick patch, although I also noticed there is a long discussion around your previous commit of removing ConstantExpr for alias. :-) Personally I'm really hoping your alias offset support patch can go into trunk as soon as possible, and then recover my global merge patch.</div>
<div><br></div><div>Thanks,</div><div>-Jiangning</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">

<br>
Cheers,<br>
Rafael<br>
</blockquote></div><br></div></div>