<html 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 15 (filtered medium)">
<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:DengXian;
panose-1:2 1 6 0 3 1 1 1 1 1;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:"\@DengXian";
panose-1:2 1 6 0 3 1 1 1 1 1;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
span.EmailStyle18
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:windowtext;}
.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>
</head>
<body lang="EN-US" link="blue" vlink="purple" style="word-wrap:break-word">
<div class="WordSection1">
<p class="MsoNormal">Thanks working on including PGOOptions in TargetMachine. It sounds good to me to leave EnableFSDiscrinator as an llvm switch for now.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Thanks,<o:p></o:p></p>
<p class="MsoNormal">Hongtao<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal" style="mso-margin-top-alt:0in;margin-right:0in;margin-bottom:12.0pt;margin-left:.5in">
<b><span style="font-size:12.0pt;color:black">From: </span></b><span style="font-size:12.0pt;color:black">Rong Xu <xur@google.com><br>
<b>Date: </b>Wednesday, August 18, 2021 at 10:15 AM<br>
<b>To: </b>Hongtao Yu <reviews+D107878+public+a10d6f6e79d09f89@reviews.llvm.org><br>
<b>Cc: </b>wmi@google.com <wmi@google.com>, Wenlei He <wenlei@fb.com>, Hongtao Yu <hoy@fb.com>, stevenwu@apple.com <stevenwu@apple.com>, matthew.voss@sony.com <matthew.voss@sony.com>, mgorny@gentoo.org <mgorny@gentoo.org>, davidxl@google.com <davidxl@google.com>,
pengfei.wang@intel.com <pengfei.wang@intel.com>, dexonsmith@apple.com <dexonsmith@apple.com>, llvm-commits@lists.llvm.org <llvm-commits@lists.llvm.org>, bhuvanendra.kumarn@amd.com <bhuvanendra.kumarn@amd.com>, yanliang.mu@intel.com <yanliang.mu@intel.com>,
dougpuob@gmail.com <dougpuob@gmail.com>, david.green@arm.com <david.green@arm.com>, yuanfang.chen@sony.com <yuanfang.chen@sony.com>, ruiling.song@amd.com <ruiling.song@amd.com><br>
<b>Subject: </b>Re: [PATCH] D107878: [SampleFDO] Flow Sensitive Sample FDO (FSAFDO) profile loader<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal" style="margin-left:.5in">I just sent out the patch that puts PGOOptions to TargetMachine before getting this message.<o:p></o:p></p>
<div>
<p class="MsoNormal" style="margin-left:.5in">So EnableFSDiscriminator is not included.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="margin-left:.5in"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal" style="margin-left:.5in">If we want to include EnableFSDiscrinator there, we'd better to promote this to an user visible option (as of now, it's an internal option).<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="margin-left:.5in">I was thinking this option only serves in the transitional phrase. In the long term, we will make it default and get rid of it.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="margin-left:.5in">So, I did not add it as a user level flag. <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="margin-left:.5in"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal" style="margin-left:.5in">If people think we should keep it, i'd happy to change it to a user level flag and include it in PGOOptions.<o:p></o:p></p>
</div>
</div>
<p class="MsoNormal" style="margin-left:.5in"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal" style="margin-left:.5in">On Wed, Aug 18, 2021 at 9:28 AM Hongtao Yu via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<p class="MsoNormal" style="mso-margin-top-alt:0in;margin-right:0in;margin-bottom:12.0pt;margin-left:.5in">
hoy added inline comments.<br>
<br>
<br>
================<br>
Comment at: llvm/include/llvm/Target/TargetMachine.h:113-116<br>
+ /// FSProfile file name.<br>
+ std::string FSProfileFile = "";<br>
+ /// FSProifle remapping file name.<br>
+ std::string FSRemappingFile = "";<br>
----------------<br>
xur wrote:<br>
> wmi wrote:<br>
> > Feel it is better to move the whole PGOOpt here. Similar as OptLevel above, PGOOpt shows us the major PGO related configuration and it will be available for the MIR passes after it is included in TargetMachine class.
<br>
> I'm OK with that. But for now, only these two fields are used in MIR (at lease for this patch).<br>
> I will make the change if I don't hear objection.<br>
Moving `PGOOpt` here sounds good to time. It may be extended for MIR profiling use in the future. Maybe the `EnableFSDiscriminator` switch can be added into `PGOOpt` as well?<br>
<br>
<br>
CHANGES SINCE LAST ACTION<br>
<a href="https://reviews.llvm.org/D107878/new/" target="_blank">https://reviews.llvm.org/D107878/new/</a><br>
<br>
<a href="https://reviews.llvm.org/D107878" target="_blank">https://reviews.llvm.org/D107878</a><o:p></o:p></p>
</blockquote>
</div>
</div>
</body>
</html>