<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Feb 1, 2016 at 12:06 PM, Matt Arsenault <span dir="ltr"><<a href="mailto:Matthew.Arsenault@amd.com" target="_blank">Matthew.Arsenault@amd.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"><div><div class="h5">
<div>On 02/01/2016 10:12 AM, David Blaikie
wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr"><br>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Mon, Feb 1, 2016 at 9:51 AM, Matt
Arsenault via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank"></a><a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">arsenm
created this revision.<br>
arsenm added a reviewer: tstellarAMD.<br>
arsenm added a subscriber: llvm-commits.<br>
Herald added a subscriber: arsenm.<br>
<br>
<a href="http://reviews.llvm.org/D16774" rel="noreferrer" target="_blank">http://reviews.llvm.org/D16774</a><br>
<br>
Files:<br>
lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp<br>
<br>
Index: lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp<br>
===================================================================<br>
--- lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp<br>
+++ lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp<br>
@@ -402,7 +402,10 @@<br>
static bool collectUsesWithPtrTypes(Value *Val,
std::vector<Value*> &WorkList) {<br>
bool Success = true;<br>
for (User *User : Val->users()) {<br>
- if(std::find(WorkList.begin(), WorkList.end(), User)
!= WorkList.end())<br>
+ if (!Success)<br>
+ return false;<br>
+<br>
+ if (std::find(WorkList.begin(), WorkList.end(), User)
!= WorkList.end())<br>
continue;<br>
if (CallInst *CI = dyn_cast<CallInst>(User)) {<br>
// TODO: We might be able to handle some cases
where the callee is a<br>
@@ -429,10 +432,10 @@<br>
continue;<br>
<br>
WorkList.push_back(User);<br>
-<br>
- Success &= collectUsesWithPtrTypes(User,
WorkList);<br>
+ Success = collectUsesWithPtrTypes(User, WorkList);<br>
</blockquote>
<div><br>
</div>
<div>Why not just drop the Success variable entirely and
return from here:<br>
<br>
if (!collectUsesWithPtrTypes(...))<br>
return false<br>
</div>
</div>
</div>
</div>
</blockquote></div></div>
Return here is what I tried first, but this breaks some cases due to
the !isPointerTy check above. Call users without pointer types would
then fall through and incorrectly be changed. </div></blockquote><div><br></div><div>Sorry, couldn't really follow... the original code would return false if collectUsesWithPtrTypes ever returns false, right? Were there side effects that were desired after that point?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div bgcolor="#FFFFFF" text="#000000">I'm planning on fixing
some of the intrinsic handling here, so maybe after that this will
be OK.<span class=""><br>
<br>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div><br>
Or, possibly better yet:<br>
<br>
return llvm::all_of(Val->users(), [&](User *User) {<br>
...<br>
return collectUsesWithPtrTypes(User, WorkList);<br>
});</div>
<div> <br>
</div>
</div>
</div>
</div>
</blockquote>
<br>
<br>
</span></div>
</blockquote></div><br></div></div>