[llvm-commits] [llvm] r143600 - in /llvm/trunk: lib/CodeGen/MachineInstr.cpp test/CodeGen/X86/MachineLICM-HoistSelectorRefs.ll

[Evan Cheng]

On Nov 3, 2011, at 11:46 AM, Peter Cooper wrote:

> 
> On Nov 3, 2011, at 11:26 AM, Chris Lattner wrote:
> 
>> On Nov 3, 2011, at 10:18 AM, Peter Cooper wrote:
>>> Hi Chris
>>> 
>>> I agree that a new linkage type is better, but that seemed overkill when there's currently only one case where this occurs.  But yes, special casing by name is particularly gross.
>> 
>> This sort of thing is an instance of a more general problem.  I can imagine other front ends that want to unique strings for example, or do load-time optimizations that prevent something from being marked truly constant.  Marking the load as "runtime constant load" through metadata seems a more general way to model this than by introducing a new linkage type on the global, but perhaps I'm missing something.
> 
> I did mention metadata to Evan but he'd said that might be too heavyweight a solution.  But there also seems to be reasons against adding a whole new linkage type too, primarily that there's quite a lot of them for developers to deal with already.

My concern is there is no precedence for using metadata annotation for scalar optimizations.

Evan

>> 
>>> I did have a solution working in the earlier LICM pass but it then struggles to do CSE on these loads.  It also meant putting my special case code in 2 places which would have been even more gross.  If i change it to a new linkage type then i'll make sure and have earlyCSE and LICM work on it too.
>> 
>> Can you elaborate on what the problem was?  
> 
> Yeah, so i found that other constant loads are optimized into one load in GVN (not always earlyCSE, got that wrong here).  So the problem was that GVN gets its information for the load from MemoryDependenceAnalysis which calls into AA.  But if i change AA to say that this global is sort of constant then later optimizations substitute the load from the global for the value it contains which is incorrect.
> 
> The test i had was this, and the problem was the call killing the first load and so stopping it from CSE with the second load.
> 
> %2 = load i8** @"\01L_OBJC_SELECTOR_REFERENCES_"
> %call = call i8* bitcast (i8* (i8*, i8*, ...)* @objc_msgSend to i8* (i8*, i8*)*)(i8* %1, i8* %2)
> %3 = load i8** @"\01L_OBJC_SELECTOR_REFERENCES_"
> 
> Pete