<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<br>
<div class="moz-cite-prefix">On 07/17/2014 02:46 PM, Chandler
Carruth wrote:<br>
</div>
<blockquote
cite="mid:CAGCO0KgK7uGvizXfdjHOBLJ2J-pGzTcUcvNXCyaS=iXqerSdxg@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="gmail_extra"><br>
<div class="gmail_quote">On Thu, Jul 17, 2014 at 5:38 PM,
Philip Reames <span dir="ltr"><<a moz-do-not-send="true"
href="mailto:listmail@philipreames.com" target="_blank">listmail@philipreames.com</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="">
<div>On 07/17/2014 01:51 PM, Chandler Carruth wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_extra"><br>
<div class="gmail_quote"><br>
<blockquote class="gmail_quote" style="margin:0
0 0 .8ex;border-left:1px #ccc
solid;padding-left:1ex">
<div style="overflow:hidden"> 2. Would adding
a canonicalization of if(c) { unreachable }
to llvm.invariant(c) would be worthwhile?<br>
</div>
</blockquote>
<div><br>
</div>
<div>There was a long and painful attempt to
implement invariants based on the
branch-to-unreachable pattern. It didn't work.
I don't expect these patterns to show up often
organically and to go away too soon in the
optimizer to be useful. The whole point of
'llvm.invariant' instead of the if construct
is to distinguish between the case the
optimizer should try to remove (a branch to
unreachable) and the case the optimizer should
try to preserve because of some specific
utility. We shouldn't lose this important
distinction.</div>
</div>
</div>
</div>
</blockquote>
</div>
On first thought, I disagree. I may not be understanding
your point though.<br>
<br>
My understanding of the previous work was that it tried to
use "branch-to-unreachable" as the canonical form. This
is inherently problematic in an IR based on basic blocks
since it split basic blocks into many smaller chunks. It
might work out better if we used extended basic blocks,
but we don't. <br>
<br>
I don't see any harm in canonicalizing to "llvm.invariant"
from "if(c) unreachable". In either case, we remove the
branch and can merge the basic blocks. In the former, we
preserve more information for later passes. The only real
downside is potentially keeping more Values alive and thus
forcing the compiler to do more work. <br>
<br>
Can you spell out your objections more?</blockquote>
</div>
<br>
Adding an invariant has a cost. Just because we nuked a branch
doesn't mean that the condition feeding the branch is
definitely a high-value invariant to preserve in the IR.</div>
<div class="gmail_extra"><br>
</div>
<div class="gmail_extra">I wouldn't mind an experiment to see if
this ended up being profitable, but I'm dubious about it's
profitability so far, and would want to see data to back up
this change as I worry about significantly increasing the
density of invariants (and thus their compile time cost and
cost on hasOneUse failures) without any real benefit.</div>
</div>
</blockquote>
Given your explanation in the other response, your position makes a
lot more sense. I agree it makes sense to run the experiment, but
for now, I'm happy accepting your position as a reasonable default.
<br>
<br>
I find myself wondering if we might want to remove invariants
somewhat aggressively. Thinking about exactly when this would be
profitably will require some serious thought through. It doesn't
appear to have an obvious "right" answer. <br>
<br>
Philip<br>
<br>
<br>
<br>
</body>
</html>