<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Aug 26, 2015, at 6:41 PM, Philip Reames <<a href="mailto:listmail@philipreames.com" class="">listmail@philipreames.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class="">
<meta content="text/html; charset=windows-1252" http-equiv="Content-Type" class="">
<div bgcolor="#FFFFFF" text="#000000" class="">
<br class="">
<br class="">
<div class="moz-cite-prefix">On 08/26/2015 04:44 PM, Pete Cooper
wrote:<br class="">
</div>
<blockquote cite="mid:1DBC309A-F4DD-4DDD-85BE-720EA362A036@apple.com" type="cite" class="">
<div class="moz-text-html" lang="x-western">Hi Philip
<div class=""><br class="">
</div>
<div class="">This is an offshoot of the conversation we had
here: <a moz-do-not-send="true" href="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=" class="">http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20150824/295815.html</a></div>
<div class=""><br class="">
</div>
<div class="">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.</div>
<div class=""><br class="">
</div>
<div class="">Thanks,</div>
<div class="">Pete</div>
<div class=""><br class="">
</div>
</div>
<br class="">
<fieldset class="mimeAttachmentHeader"><legend class="mimeAttachmentHeaderName">addrspace-nullness.patch</legend></fieldset>
<br class="">
<div class="moz-text-plain" wrap="true" graphical-quote="true" style="font-family: -moz-fixed; font-size: 12px;" lang="x-western">
<pre wrap="" class="">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.</pre>
</div>
</blockquote>
The actual code change is fine, but the comment is inaccurate. You
dropped the bit about extern weak.<br class=""></div></div></blockquote>Good point.<br class=""><blockquote type="cite" class=""><div class=""><div bgcolor="#FFFFFF" text="#000000" class="">
<br class="">
Possibly something along the lines of:<br class="">
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.<br class=""></div></div></blockquote>Yeah, thats much better. Committed as r246137 with your exact wording.</div><div><br class=""></div><div>Thanks for the review.</div><div>Pete<br class=""><blockquote type="cite" class=""><div class=""><div bgcolor="#FFFFFF" text="#000000" class="">
<blockquote cite="mid:1DBC309A-F4DD-4DDD-85BE-720EA362A036@apple.com" type="cite" class="">
<div class="moz-text-plain" wrap="true" graphical-quote="true" style="font-family: -moz-fixed; font-size: 12px;" lang="x-western">
<pre wrap="" class=""> 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.
+
+@as0 = external global i32
+@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
+}
</pre>
</div>
</blockquote>
<br class="">
</div>
</div></blockquote></div><br class=""></body></html>