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

[Peter Cooper]

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.  

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.

Pete

On Nov 3, 2011, at 9:38 AM, Chris Lattner wrote:

> 
> On Nov 2, 2011, at 5:56 PM, Pete Cooper wrote:
> 
>> Author: pete
>> Date: Wed Nov  2 19:56:36 2011
>> New Revision: 143600
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=143600&view=rev
>> Log:
>> Treat objc selector reference globals as invariant so that MachineLICM can hoist them out of loops.  Fixes <rdar://problem/6027699>
> 
> Hi Pete,
> 
> Special casing this by-name seems really gross.  Shouldn't this be handled by introducing a new linkage type, or some sort of metadata?  Also, would it be possible to catch this in the early LICM pass?
> 
> -Chris
> 
>>   llvm/trunk/test/CodeGen/X86/MachineLICM-HoistSelectorRefs.ll
>> Modified:
>>   llvm/trunk/lib/CodeGen/MachineInstr.cpp
>> 
>> Modified: llvm/trunk/lib/CodeGen/MachineInstr.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineInstr.cpp?rev=143600&r1=143599&r2=143600&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/MachineInstr.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/MachineInstr.cpp Wed Nov  2 19:56:36 2011
>> @@ -1232,6 +1232,19 @@
>>  return false;
>> }
>> 
>> +/// pointsToRuntimeConstantMemory - Return true if this value points to data
>> +/// which does never changes once the program starts running
>> +static bool pointsToRuntimeConstantMemory(const Value *V) {
>> +  if (const GlobalVariable *GV = dyn_cast<GlobalVariable>(V)) {
>> +    StringRef Name = GV->getName();
>> +    // These special values are known to be constant at runtime
>> +    // TODO: a new linkage type for these would be far better than this check
>> +    if (Name.startswith("\01L_OBJC_SELECTOR_REFERENCES_"))
>> +      return true;
>> +  }
>> +  return false;
>> +}
>> +
>> /// isInvariantLoad - Return true if this instruction is loading from a
>> /// location whose value is invariant across the function.  For example,
>> /// loading a value from the constant pool or from the argument area
>> @@ -1259,6 +1272,8 @@
>>      if (const PseudoSourceValue *PSV = dyn_cast<PseudoSourceValue>(V))
>>        if (PSV->isConstant(MFI))
>>          continue;
>> +      if (pointsToRuntimeConstantMemory(V))
>> +        continue;
>>      // If we have an AliasAnalysis, ask it whether the memory is constant.
>>      if (AA && AA->pointsToConstantMemory(
>>                      AliasAnalysis::Location(V, (*I)->getSize(),
>> 
>> Added: llvm/trunk/test/CodeGen/X86/MachineLICM-HoistSelectorRefs.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/MachineLICM-HoistSelectorRefs.ll?rev=143600&view=auto
>> ==============================================================================
>> --- llvm/trunk/test/CodeGen/X86/MachineLICM-HoistSelectorRefs.ll (added)
>> +++ llvm/trunk/test/CodeGen/X86/MachineLICM-HoistSelectorRefs.ll Wed Nov  2 19:56:36 2011
>> @@ -0,0 +1,30 @@
>> +; RUN: llc < %s -march=x86-64 -mattr=+sse3,+sse41 -stats |& grep {1 machine-licm}
>> +; rdar://6027699
>> +
>> +target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
>> +target triple = "x86_64-apple-macosx10.7.2"
>> +
>> +; STATS: 1 machine-licm
>> +
>> +@"\01L_OBJC_METH_VAR_NAME_" = internal global [4 x i8] c"foo\00", section "__TEXT,__objc_methname,cstring_literals", align 1
>> +@"\01L_OBJC_SELECTOR_REFERENCES_" = internal global i8* getelementptr inbounds ([4 x i8]* @"\01L_OBJC_METH_VAR_NAME_", i64 0, i64 0), section "__DATA, __objc_selrefs, literal_pointers, no_dead_strip"
>> +@"\01L_OBJC_IMAGE_INFO" = internal constant [2 x i32] [i32 0, i32 16], section "__DATA, __objc_imageinfo, regular, no_dead_strip"
>> + at llvm.used = appending global [3 x i8*] [i8* getelementptr inbounds ([4 x i8]* @"\01L_OBJC_METH_VAR_NAME_", i32 0, i32 0), i8* bitcast (i8** @"\01L_OBJC_SELECTOR_REFERENCES_" to i8*), i8* bitcast ([2 x i32]* @"\01L_OBJC_IMAGE_INFO" to i8*)], section "llvm.metadata"
>> +
>> +define void @test(i8* %x) uwtable ssp {
>> +entry:
>> +  br label %for.body
>> +
>> +for.body:                                         ; preds = %for.body, %entry
>> +  %i.01 = phi i32 [ 0, %entry ], [ %inc, %for.body ]
>> +  %0 = load i8** @"\01L_OBJC_SELECTOR_REFERENCES_", align 8
>> +  %call = tail call i8* bitcast (i8* (i8*, i8*, ...)* @objc_msgSend to i8* (i8*, i8*)*)(i8* %x, i8* %0)
>> +  %inc = add i32 %i.01, 1
>> +  %exitcond = icmp eq i32 %inc, 10000
>> +  br i1 %exitcond, label %for.end, label %for.body
>> +
>> +for.end:                                          ; preds = %for.body
>> +  ret void
>> +}
>> +
>> +declare i8* @objc_msgSend(i8*, i8*, ...) nonlazybind
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>