<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=windows-1251">
<meta content="text/html; charset=utf-8">
</head>
<body>
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style>
<!--
@font-face
{font-family:"Cambria Math"}
@font-face
{font-family:Calibri}
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0cm;
font-size:11.0pt;
font-family:"Calibri",sans-serif}
a:link, span.MsoHyperlink
{color:blue;
text-decoration:underline}
p.MsoListParagraph, li.MsoListParagraph, div.MsoListParagraph
{margin-top:0cm;
margin-right:0cm;
margin-bottom:0cm;
margin-left:36.0pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif}
.MsoChpDefault
{}
@page WordSection1
{margin:72.0pt 72.0pt 72.0pt 72.0pt}
div.WordSection1
{}
ol
{margin-bottom:0cm}
ul
{margin-bottom:0cm}
-->
</style>
<div class="WordSection1">
<p class="MsoNormal"><span lang="EN-US">My 2 cents:</span></p>
<p class="MsoNormal"><span lang="EN-US"> </span></p>
<ol start="1" type="1" style="margin-top:0cm">
<li class="MsoListParagraph" style="margin-left:0cm"><span lang="EN-US">2% of code size improvement is alone a strong argument to revert r160529. In embedded world it sometimes determines whether image fits the ROM or not.</span></li></ol>
<p class="MsoListParagraph"> </p>
<ol start="2" type="1" style="margin-top:0cm">
<li class="MsoListParagraph" style="margin-left:0cm"><span lang="EN-US">Code/data size reduction often correlates with performance improvements, due to smaller number of cache refills. I suspect data segment was reduced somewhat more than those 2%.</span></li></ol>
<p class="MsoNormal"> </p>
<ol start="3" type="1" style="margin-top:0cm">
<li class="MsoListParagraph" style="margin-left:0cm"><span lang="EN-US">I don’t use LSAN and the whole idea that “what is reachable is not a leak” isn’t applicable to my use cases quite well. There are other ways to find leaking memory which do not prevent
compiler optimizations. I suspect many people also don’t use LSAN, why should they suffer?</span></li></ol>
<p class="MsoNormal"> </p>
<ol start="4" type="1" style="margin-top:0cm">
<li class="MsoListParagraph" style="margin-left:0cm"><span lang="EN-US">As it was already mentioned patch introduced by r160529 is of substandard quality and is not properly covered by test cases. This is yet another argument for removal.</span></li></ol>
<p class="MsoListParagraph"> </p>
<ol start="5" type="1" style="margin-top:0cm">
<li class="MsoListParagraph" style="margin-left:0cm"><span lang="EN-US">I still can’t understand why LSAN instrumentation can’t prevent globals from being removed, delegating this to globalopt instead. Can someone explain this to me, please?</span></li></ol>
<p class="MsoNormal"> </p>
<p class="MsoNormal"> </p>
<div style="border:none; border-top:solid #E1E1E1 1.0pt; padding:3.0pt 0cm 0cm 0cm">
<p class="MsoNormal" style="border:none; padding:0cm"><b>From: </b><a href="mailto:llvm-dev@lists.llvm.org">Sterling Augustine via llvm-dev</a><br>
<b>Sent: </b>23 àïðåëÿ 2021 ã. 3:44<br>
<b>To: </b><a href="mailto:clattner@nondot.org">Chris Lattner</a><br>
<b>Cc: </b><a href="mailto:llvm-dev@lists.llvm.org">llvm-dev</a><br>
<b>Subject: </b>[EXTERNAL] Re: [llvm-dev] Eliminating global memory roots (or not) to help leak checkers</p>
</div>
<p class="MsoNormal"> </p>
</div>
<div>
<div style="font-size:9pt; font-family:'Calibri',sans-serif">
<h3 style="background-color:#ffffff; font-size:10pt; border:1px dotted #003333; padding:.8em">
<span style="color:#ff6600">CAUTION:<strong> </strong></span>This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. If you suspect potential phishing or spam
email, report it to ReportSpam@accesssoftek.com</h3>
</div>
<div>
<div dir="ltr">Have any of the proponents of the patch tried lsan against a reasonable large code base both with and with out it?</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Thu, Apr 22, 2021 at 4:28 PM Chris Lattner <<a href="mailto:clattner@nondot.org">clattner@nondot.org</a>> wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex; border-left:1px solid rgb(204,204,204); padding-left:1ex">
<br>
<br>
> On Apr 19, 2021, at 5:24 AM, James Y Knight <<a href="mailto:jyknight@google.com" target="_blank">jyknight@google.com</a>> wrote:<br>
> <br>
> There is no problem (no leaks) in the code that users wrote, so adding code annotations (sanitizer suppression file, or attributes) is not a good solution to this issue. The problem is that this optimization introduces a "leak" (from the point of view of
the leak checker), which wasn't there before. And in practice, this seems to cause a large number of false positives.<br>
<br>
I think that “from the point of view of the leak checker” is the key thing there.<br>
<br>
Code that this triggers *is* leaking memory, it was just silenced because the leak was spuriously reachable from a global. Global variables aren’t a preferred way to silence leak detectors, they have other ways to do so :)<br>
<br>
> On Apr 19, 2021, at 4:52 PM, Sterling Augustine <<a href="mailto:saugustine@google.com" target="_blank">saugustine@google.com</a>> wrote:<br>
> <br>
> There may be other ways to disable leak checkers, but they put a burden on users that was not there before. Further, users who try leak checking with and without optimization will get different answers. The bug report will read: "clang at -O2 makes my program
leak". And, as James notes, whether or not you need to suppress the leak depends on whether or not the optimizer does away with the variable. Subtle changes to the code that have nothing to do with memory allocation will appear to add or fix leaks. That is
not something I would care to explain or document.<br>
<br>
I can see that concern, but this isn’t a battle that we can win: optimizations in general can expose leaks.<br>
<br>
IMO, If someone doesn’t want the global to be removed, they should mark it volatile. If they do want it removable, then they can use leak detector features to silence the warning.<br>
<br>
> On Apr 20, 2021, at 9:12 AM, Sterling Augustine <<a href="mailto:saugustine@google.com" target="_blank">saugustine@google.com</a>> wrote:<br>
> <br>
> In order to understand how much benefit this change gives to code size, I built clang and related files with and without the patch, both with CMAKE_BUILD_TYPE=Release.<br>
> <br>
> clang itself gets about 0.4% smaller (from 145217124 to 144631078)<br>
> lld gets about 1.85% smaller (from 101129181 to 99243810 bytes)<br>
> <br>
> Spot checking a few other random targets, in general, it seems that the benefits depend heavily on the coding style, but roughly, bigger the binary, the less benefit this brings.<br>
> <br>
> I suspect that a more aggressive pass as described by Philip Reames could capture a significant part of the benefit without sacrificing functionality. But that would require a more detailed study to be sure.
<br>
<br>
A ~2% reduction in code size is a huge win. I agree with your comment about it being different with different coding styles. I suspect that this is the sort of thing that will pay particularly for high abstraction code bases.<br>
<br>
I don’t see why we would punish general code to make “code that is leaking where formerly not detected, and where users don’t want to mark the root as volatile”. This seems really unprincipled to me, and a slippery slope we can’t go down.<br>
<br>
-Chris<br>
<br>
</blockquote>
</div>
</div>
</div>
</body>
</html>