<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<meta name="Generator" content="Microsoft Word 12 (filtered medium)">
<base href="x-msg://364/"><style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Tahoma;
panose-1:2 11 6 4 3 5 4 4 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman","serif";}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
span.apple-converted-space
{mso-style-name:apple-converted-space;}
span.EmailStyle18
{mso-style-type:personal-reply;
font-family:"Calibri","sans-serif";
color:#1F497D;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="blue" vlink="purple" style="word-wrap: break-word;-webkit-nbsp-mode: space;-webkit-line-break: after-white-space">
<div class="WordSection1">
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Hi Nick<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">The transformation is not expensive. It is early-inlining that bloats code. So each added new pass will process a significant amount of codes. I agree that
this is a design mishap inside my organization. I will re-factor the code following your suggestion. If the check-in process can’t be finished next week, please allow some delay since my sabbatical is coming.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">-Mei<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<div>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> Andrew Trick [mailto:atrick@apple.com]
<br>
<b>Sent:</b> Sunday, July 28, 2013 2:43 AM<br>
<b>To:</b> Ye, Mei<br>
<b>Cc:</b> Matt Arsenault; Evan Cheng; llvm commits; Chandler Carruth; Koenig, Christian<br>
<b>Subject:</b> Re: [patch] simplifyCFG for review<o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal">On Jul 27, 2013, at 12:17 PM, "Ye, Mei" <<a href="mailto:Mei.Ye@amd.com">Mei.Ye@amd.com</a>> wrote:<o:p></o:p></p>
</div>
<p class="MsoNormal"><br>
<br>
<o:p></o:p></p>
<div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Hi all</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"> </span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Code re-factoring and re-org to separate canonicalization and optimization sounds like a good long-term solution. These two functionalities aren’t always
well separated in existing “generic” passes. From time to time, there are demands to fine-tune “generic” pass for underlying targets, e.g., CSE can cause register pressure, strength reduction must consider target’s addressing mode. It is probably a no-win
battle to argue whether certain opts are “generic” enough.</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"> </span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">W.R.T Andy’s question on ordering of MergeIfRegion and<span class="apple-converted-space"> </span></span>SimplifyParallelAndOr, if the optimization bails out
and iterates whenever a change of CFG happens, then there is no ordering concern. If one iteration can do invoke more than one transformations (which can improve compilation time), then it makes sense to put SimplifyParallelAndOr before MergeIfRegion, since
the former can expose opportunities for the latter. An example is that the if-region is inside a loop and the loop is completely unrolled, instances of if-regions can be merged after the height of conditions are reduced.<o:p></o:p></p>
</div>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Mei,<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">It does not appear that you need to interleave your new transforms with the standard SimplifyCFG transforms. Making them a utility for use in target passes would add a lot of flexibility.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">The only issue driving the less flexible design then is compile time. I think I misunderstood the issue, which you and Christian have just clarified. It's not that SimplifyCFG pass carries a significant cost (I wouldn't expect it to), it's
actually that your transformation itself is costly, and you want to apply it before inlining.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">If my assertions above are true, then Christian's suggestion was excellent if it can be made to work. Here's a concrete proposal:<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">- Move SimplifyParallelAndOr and MergeIfRegion either into a new file Transforms/Utils/FlattenCFG.cpp. Or pick a better filename. I actually think what you're doing is branch folding, but that name is taken.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">- Expose the SimplifyParallelAndOr and MergeIfRegion in Transforms/Utils/FlattenCFG.hpp.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">- Create a simple pass in Target/R600 to run your utils on the CFG. Invoke it the target pass pipe--maybe addCodeGenPrepare(). I don't see much value in sharing the pass driver code, and it can always be factored later.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">- Measure the compile-time impact. How bad is it really? If it's a problem, your current approach won't work in the long-term anyway because we plan to move OptimizeCFG outside of CGSCC (after inlining).<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">- If you need to run FlattenCFG stuff before inlining, follow Christian's suggestion.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Incidentally, it seems logical to me that you should be able implement inlining as a function pass provided by TargetMachine. I know that's verboten in the initial passmanager, but by the time we reach codegen, all the function bodies should
be complete. Then somehow forcing a CGSCC pass manager would allow you to visit functions in call-tree bottom-up order. It's likely this is impossible though, and you'd need to split your target pass pipeline with a CGSCC inliner in the middle.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">-Andy<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">Thanks.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"> </span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">-Mei</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"> </span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"> </span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"> </span><o:p></o:p></p>
</div>
<div>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in">
<div>
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span class="apple-converted-space"><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> </span></span><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">Andrew
Trick [mailto:atrick@<a href="http://apple.com">apple.com</a>]<span class="apple-converted-space"> </span><br>
<b>Sent:</b><span class="apple-converted-space"> </span>Saturday, July 27, 2013 1:56 AM<br>
<b>To:</b><span class="apple-converted-space"> </span>Ye, Mei<br>
<b>Cc:</b><span class="apple-converted-space"> </span>Matt Arsenault; Evan Cheng; llvm commits; Chandler Carruth<br>
<b>Subject:</b><span class="apple-converted-space"> </span>Re: [patch] simplifyCFG for review</span><o:p></o:p></p>
</div>
</div>
</div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
<div>
<div>
<div>
<p class="MsoNormal">On Jul 27, 2013, at 1:30 AM, Chandler Carruth <<a href="mailto:chandlerc@google.com"><span style="color:purple">chandlerc@google.com</span></a>> wrote:<o:p></o:p></p>
</div>
</div>
<div>
<p class="MsoNormal"><br>
<br>
<br>
<o:p></o:p></p>
</div>
<div>
<div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
<div>
<div>
<p class="MsoNormal">On Sat, Jul 27, 2013 at 1:07 AM, Andrew Trick <<a href="mailto:atrick@apple.com" target="_blank"><span style="color:purple">atrick@apple.com</span></a>> wrote:<o:p></o:p></p>
</div>
<div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<p class="MsoNormal">(1) In terms of code organization, these anti-canonical, target-selected transforms should live somewhere else. I kept quiet becase I hadn't come up with an alternative, but we can do that now. OptimizeCFG.cpp?<o:p></o:p></p>
</div>
</blockquote>
<div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
</div>
</div>
<div>
<div>
<p class="MsoNormal">It would be even more self-evident to group all of these type of branch avoidance utils into a FlattenCFG package. So I'm changing my vote to FlattenCFG.cpp.<o:p></o:p></p>
</div>
</div>
</div>
<div>
<p class="MsoNormal"><br>
To focus on the immediate issue: I agree.<o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal">Design wise, I would suggest one step further: I think that CFG-flattening of this form is somewhat specialized and we should just build a nice specialized set of optimizations targetting that use case, and have the targets add those passes
from their target machine rather than monkey with the general purpose PMB. We can't easily get this right in the PMB because of the silly way CGSCC stuff is defined, and I think that this is likely to be best as a late-stage CFG pass anyways not unlike LSR,
etc. I'm not sure that it really has anything to do with SimplifyCFG (or OptimizeCFG, which I've begun to think is somewhat likely to make more sense in MI w/ register pressure and critical path info). So, my vote is for a more targeted tool in the toolbox.<o:p></o:p></p>
</div>
</div>
</div>
</div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
<div>
<div>
<p class="MsoNormal">Mei,<o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal">You seem to have a reason for running SimplifyParallelAndOr after the basic cleanup transforms but before branch simplification. Whereas MergeIfRegion runs only after all other simplifications. That does seem intuitive to me, but I wonder
if it is absolutely necessary. Can you illustrate with an example or two why the simplifications need to be interleaved this way? Are you just trying to avoid the need for another round of iteration in the SimplifyCFGPass driver? I think the direction of design
refactoring depends on it.<o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal">-Andy<o:p></o:p></p>
</div>
</div>
</div>
</blockquote>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</body>
</html>