<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body>
    <p>Don't really have an opinion on the question as asked, but want
      to point out an alternate framing.   I will comment that the code
      being removed looks more than a bit fragile.  <br>
    </p>
    <p>From a very quick skim of the original code, it appears to be
      focused on DCE of globals.  If we wanted to keep the "leak
      detector safe" semantic, but allow more aggressive optimization,
      we could re-imagine this as global SROA.  We could deconstruct the
      struct/array/etc and keep only the fields which could potentially
      be allocation roots.  We could also write an optimization which
      leverages the knowledge of the allocation root being otherwise
      unused to eliminate mallocs which are stored into them.</p>
    <p>I haven't fully thought that through, but it seems like we have
      quite a bit of room to optimize better without changing our
      handling for the leak detectors.  <br>
    </p>
    <p>Philip<br>
    </p>
    <div class="moz-cite-prefix">On 4/14/21 9:38 AM, Sterling Augustine
      via llvm-dev wrote:<br>
    </div>
    <blockquote type="cite"
cite="mid:CAEG7qUwQa4-UadsSjDwfdw7P7=01oiUogQYo0RFXR--LGL8hKQ@mail.gmail.com">
      <meta http-equiv="content-type" content="text/html; charset=UTF-8">
      <div dir="ltr">
        <div>[Continuing discussion from <a
            href="https://reviews.llvm.org/D69428"
            moz-do-not-send="true">https://reviews.llvm.org/D69428</a>]<br>
        </div>
        <div><br>
        </div>
        <div>Llvm is fairly conservative when eliminating global
          variables (or fields of such) that may point to dynamically
          allocated memory. This behavior is entirely to help leak
          checking tools such as Valgrind, Google's HeapLeakChecker, and
          LSAN, all of which treat memory that is reachable at exit as
          "not leaked", even though it will never be freed. Without
          these global variables to hold the pointer, the leak checkers
          can't determine that it is actually reachable, and will report
          a leak. Global variables that dynamically allocate memory but
          don't clean themselves up are fairly common in the wild, and
          various leak checkers have long not reported errors.</div>
        <div><br>
        </div>
        <div>This behavior was added all the way back in 2012 in <a
href="https://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20120625/145646.html"
            moz-do-not-send="true">https://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20120625/145646.html</a>.</div>
        <div><br>
        </div>
        <div><a href="https://reviews.llvm.org/D69428"
            moz-do-not-send="true">https://reviews.llvm.org/D69428</a>
          removed this behavior, and I subsequently reverted it when
          many internal Google tests started failing, but I believe many
          other users who use leak checking will encounter errors when
          this hits more mainstream releases.<br>
        </div>
        <div><br>
        </div>
        <div>So: What to do?</div>
        <div><br>
        </div>
        <div>Preventing a valid transformation (the global variables are
          never read and can be eliminated) to help the leak checkers
          leaves some performance and code size on the table. Just how
          much is unclear.</div>
        <div><br>
        </div>
        <div>On the other hand, having leak checkers suddenly start
          reporting failures where they didn't before also seems
          suboptimal. Cleaning this somewhat common scenario up is
          surprisingly difficult at the user level.</div>
        <div><br>
        </div>
        <div>Some possibilities:</div>
        <div><br>
        </div>
        <div>1. Only do this at high optimization levels, say -O3. This
          would give aggressive users all the performance we can, but
          also make leak checkers report leaks sometimes, but not
          others.</div>
        <div><br>
        </div>
        <div>2. Hide it behind a flag or configurable option. Users who
          care can set it as they prefer. Creates more confusing
          options, different testing matrices and such, but everyone can
          get the behaviour that they want.</div>
        <div><br>
        </div>
        <div>3. Do it all the time, and users who encounter issues can
          clean up their code. Users get the most performance they
          possibly can, but have to clean up code or drop leak checking.
          Seems a little user hostile.</div>
        <div><br>
        </div>
        <div>Other possibilities?:</div>
      </div>
      <br>
      <fieldset class="mimeAttachmentHeader"></fieldset>
      <pre class="moz-quote-pre" wrap="">_______________________________________________
LLVM Developers mailing list
<a class="moz-txt-link-abbreviated" href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>
<a class="moz-txt-link-freetext" href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a>
</pre>
    </blockquote>
  </body>
</html>