[PATCH] Globals in non-zero address spaces might be null

[Pete Cooper via llvm-commits]

> On Aug 26, 2015, at 6:41 PM, Philip Reames <listmail at philipreames.com> wrote:
> 
> 
> 
> On 08/26/2015 04:44 PM, Pete Cooper wrote:
>> Hi Philip
>> 
>> This is an offshoot of the conversation we had here: http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20150824/295815.html <https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.llvm.org_pipermail_llvm-2Dcommits_Week-2Dof-2DMon-2D20150824_295815.html&d=BQMD-g&c=eEvniauFctOgLOKGJOplqw&r=03tkj3107244TlY4t3_hEgkDY-UG6gKwwK0wOUS3qjM&m=ey_hdHju-ren_LsxQdOgdMDoyqlRSQjcfg1MH6sEiWg&s=MF3P42sMwfU-K0u_cMGUXvvamIAwSY_XB1SPqiPLobs&e=>
>> 
>> The attached patch make sure that we only add nonnull attributes to globals in address space 0.  Other address spaces may or may not permit a global to live at address 0, but we have to assume that they might.
>> 
>> Thanks,
>> Pete
>> 
>> 
>> addrspace-nullness.patch
>> 
>> diff --git a/lib/Analysis/ValueTracking.cpp b/lib/Analysis/ValueTracking.cpp
>> index 9b930d7..71fe1e6 100644
>> --- a/lib/Analysis/ValueTracking.cpp
>> +++ b/lib/Analysis/ValueTracking.cpp
>> @@ -3227,8 +3227,12 @@ bool llvm::isKnownNonNull(const Value *V, const TargetLibraryInfo *TLI) {
>>      return A->hasByValOrInAllocaAttr() || A->hasNonNullAttr();
>>  
>>    // Global values are not null unless extern weak.
>> +  // Note that only globals in address space 0 are guaranteed to not be null.
>> +  // Other address spaces are potentially able to have null addresses, so we
>> +  // have to be safe here and assume that is the case.
> The actual code change is fine, but the comment is inaccurate.  You dropped the bit about extern weak.
Good point.
> 
> Possibly something along the lines of:
> A global variable in address space 0 is non null unless extern weak.  Other address spaces may have null as a valid address for a global, so we can't assume anything.
Yeah, thats much better.  Committed as r246137 with your exact wording.

Thanks for the review.
Pete
>>    if (const GlobalValue *GV = dyn_cast<GlobalValue>(V))
>> -    return !GV->hasExternalWeakLinkage();
>> +    return !GV->hasExternalWeakLinkage() &&
>> +           GV->getType()->getAddressSpace() == 0;
>>  
>>    // A Load tagged w/nonnull metadata is never null. 
>>    if (const LoadInst *LI = dyn_cast<LoadInst>(V))
>> diff --git a/test/Transforms/InstCombine/nonnull-attribute.ll b/test/Transforms/InstCombine/nonnull-attribute.ll
>> new file mode 100644
>> index 0000000..74fb091
>> --- /dev/null
>> +++ b/test/Transforms/InstCombine/nonnull-attribute.ll
>> @@ -0,0 +1,19 @@
>> +; RUN: opt < %s -instcombine -S | FileCheck %s
>> +
>> +; This test makes sure that we do not assume globals in address spaces other
>> +; than 0 are able to be null.
>> +
>> + at as0 = external global i32
>> + at as1 = external addrspace(1) global i32
>> +
>> +declare void @addrspace0(i32*)
>> +declare void @addrspace1(i32 addrspace(1)*)
>> +
>> +; CHECK: call void @addrspace0(i32* nonnull @as0)
>> +; CHECK: call void @addrspace1(i32 addrspace(1)* @as1)
>> +
>> +define void @test() {
>> +  call void @addrspace0(i32* @as0)
>> +  call void @addrspace1(i32 addrspace(1)* @as1)
>> +  ret void
>> +}
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150826/996c6446/attachment.html>