<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Feb 1, 2016 at 1:13 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"><span class="">
    <div>On 02/01/2016 12:48 PM, David Blaikie
      wrote:<br>
    </div>
    <blockquote type="cite">
      <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>
              <blockquote type="cite">
                <div dir="ltr">
                  <div class="gmail_extra">
                    <div class="gmail_quote">
                      <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><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>
    </blockquote>
    <br></span>
    collectUsesWithPtrTypes should only be called with a pointer type
    value. Assuming there is an intrinsic call that uses a pointer and
    returns an integer for example, the
    User->getType()->isPointerTy() check will cause the call user
    to not be added to the worklist, because we do not want to have
    non-pointers in the worklist. I should probably just add another
    pointer check under the call handling so it stops falling through to
    here.<br></div></blockquote><div><br>Sorry, think I'm talking past you/not making sense. I haven't looked/understood the actual purpose of this loop - I'm just looking at it mechanically.<br><br>The original code was:<br><br>  for (element) {<br>    if (!work(element))</div><div>      continue;<br>    Success &= func(element);<br>  }<br>  return Success;<br><br>So, previously work was called for every element, and if func(element) ever returned false for a relevent (where work returns true) , the function returned false (otherwise true)<br><br>The new code you have is:<br><br>  for (element) {<br>    if (!Success)<br>      return false;</div><div>    if (!work(element))<br>      continue;<br>    Success = func(element);<br>  }<br>  return true;<br><br>This has the same behavior as the original code, except for the final iteration - if func(element) returns false on the last iteration through the loop, the function will now return true instead of false. Is that intended? (if so, a test would be good) or is it just an impossible state?<br><br>Why wait for the next run around the loop to check Success and return? Rather than just returning immediately:<br><br>  for (element) {<br>    if (!work(element))<br>      continue;<br>    if (!func(element))<br>      return false;<br>  }<br>  return true;<br><br>This has the same behavior as the original code (doesn't have the change in behavior for the last iteration through the loop). If that last iteration isn't important (the difference in behavior isn't intentional or important (because it never comes up that way, etc)) this seems simpler - the only difference then is that one returns immediately, the other increments the for loop iterator once more, then returns - which doesn't seem important?<br><br>- Dave</div></div><br></div></div>