<div dir="ltr">One of your run lines doesn't do anything:<div><br><div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only %s</blockquote><div><br></div><div>Please remove it. </div></div></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, May 1, 2015 at 12:10 PM, Li, Charles <span dir="ltr"><<a href="mailto:charles_li@playstation.sony.com" target="_blank">charles_li@playstation.sony.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div lang="EN-US" link="blue" vlink="purple">
<div>
<p class="MsoNormal"><span style="font-family:"Arial","sans-serif";color:black">Hi David,<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-family:"Arial","sans-serif";color:black"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-family:"Arial","sans-serif";color:black">I have revised the TLS Max Align patch taken into account of all the reviews from you and Richard.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-family:"Arial","sans-serif";color:black">I would very much appreciate another code review.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-family:"Arial","sans-serif";color:black"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-family:"Arial","sans-serif";color:black">Also Richard has the following question for you from the previous email (review for the 3<sup>rd</sup> revision of the patch). I would also appreciate your input.<u></u><u></u></span></p><span class="">
<p class="MsoNormal" style="margin-left:.5in"><span style="font-family:"Arial","sans-serif";color:#215868">> David: You have the most recent state on this -- can you check this is doing the right set of checks to find a dependent alignment? (Also, are we doing
this check elsewhere? Should we move this check to ASTContext?)</span><span style="font-family:"Arial","sans-serif";color:black"><u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-family:"Arial","sans-serif";color:black"><u></u> <u></u></span></p>
</span><p class="MsoNormal"><span style="font-family:"Arial","sans-serif";color:black">Thank you,<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-family:"Arial","sans-serif";color:black">Charles Li<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></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""> Li, Charles
<br>
<b>Sent:</b> Thursday, April 30, 2015 6:48 PM<br>
<b>To:</b> <a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a>; 'Richard Smith'; David Majnemer<br>
<b>Cc:</b> Domizioli, Dario<br>
<b>Subject:</b> RE: Proposed patch (4th revision) adds TLS Max Alignment diagnostic<u></u><u></u></span></p>
</div>
</div><div><div class="h5">
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal"><span style="font-family:"Arial","sans-serif";color:black">Hi Richard,<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-family:"Arial","sans-serif";color:black"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-family:"Arial","sans-serif";color:black">Thank you very much for pointing out all the intricacies I was not aware of.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-family:"Arial","sans-serif";color:black">Both points have been addressed.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-family:"Arial","sans-serif";color:black"><u></u> <u></u></span></p>
<p><u></u><span style="font-family:"Arial","sans-serif";color:black"><span>1.<span style="font:7.0pt "Times New Roman"">
</span></span></span><u></u><span style="font-family:"Arial","sans-serif";color:black">Dependent type check has been moved into the function
</span><span style="font-family:Consolas;color:black">hasDependentAlignment</span><span style="font-family:"Arial","sans-serif";color:black"><u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-family:"Arial","sans-serif";color:black"><u></u> <u></u></span></p>
<p><u></u><span style="font-family:"Arial","sans-serif";color:black"><span>2.<span style="font:7.0pt "Times New Roman"">
</span></span></span><u></u><span style="font-family:"Arial","sans-serif";color:black">The previous comment<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:1.0in"><span style="font-family:Consolas;color:black">/// hasDependentAlignment - determines if a variable's alignment is dependent<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:.5in"><span style="font-family:"Arial","sans-serif";color:black">has been changed to<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:1.0in"><span style="font-family:Consolas;color:black">/// \brief Determines if a variable's alignment is dependent.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-family:"Arial","sans-serif";color:black"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-family:"Arial","sans-serif";color:black">Please let me know if there are any more bugs. I very much appreciate it.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-family:"Arial","sans-serif";color:black"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-family:"Arial","sans-serif";color:black">Sincerely,<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-family:"Arial","sans-serif";color:black">Charles Li<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<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"">
<a href="mailto:metafoo@gmail.com" target="_blank">metafoo@gmail.com</a> [<a href="mailto:metafoo@gmail.com" target="_blank">mailto:metafoo@gmail.com</a>]
<b>On Behalf Of </b>Richard Smith<br>
<b>Sent:</b> Thursday, April 30, 2015 4:43 PM<br>
<b>To:</b> Li, Charles; David Majnemer<br>
<b>Cc:</b> <a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a>; Domizioli, Dario<br>
<b>Subject:</b> Re: Proposed patch (3rd revision) adds TLS Max Alignment diagnostic<u></u><u></u></span></p>
</div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<p class="MsoNormal">Looks OK to me, but I'd prefer the VD->getType()->isDependentType() check to be inside hasDependentAlignment, since a variable with a dependent type is assumed to have a dependent alignment.<u></u><u></u></p>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">David: You have the most recent state on this -- can you check this is doing the right set of checks to find a dependent alignment? (Also, are we doing this check elsewhere? Should we move this check to ASTContext?)<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">One totally trivial thing to fix before commit:<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">+/// hasDependentAlignment - determines if a variable's alignment is dependent<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">We prefer to use \brief instead of repeating the function name in new code.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">Thanks!<u></u><u></u></p>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<p class="MsoNormal">On Thu, Apr 30, 2015 at 4:21 PM, Li, Charles <<a href="mailto:charles_li@playstation.sony.com" target="_blank">charles_li@playstation.sony.com</a>> wrote:<u></u><u></u></p>
<div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Hi Everyone,</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">I am back again with the 3<sup>rd</sup> revision of the patch which adds an error diagnostic for
when TLS variables exceed maximum TLS alignment.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">This patch fixed 4 shortcomings.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">1.</span><span style="font-size:7.0pt;color:#1f497d">
</span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Dependent alignment check has been spun off into a static function called hasDependentAlignment</span><u></u><u></u></p>
<p><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">2.</span><span style="font-size:7.0pt;color:#1f497d">
</span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Redundant code on checking for reference type and getting its pointee type has been removed.</span><u></u><u></u></p>
<p><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">3.</span><span style="font-size:7.0pt;color:#1f497d">
</span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Improved diagnostic to show the requested alignment of the variable</span><u></u><u></u></p>
<p><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">4.</span><span style="font-size:7.0pt;color:#1f497d">
</span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Added a Run line in the test to ensure x86_64-linux-gnu works the same as before.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Many thanks to the peer reviewers out there.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Sincerely,</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Charles Li</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<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"">
<a href="mailto:metafoo@gmail.com" target="_blank">metafoo@gmail.com</a> [mailto:<a href="mailto:metafoo@gmail.com" target="_blank">metafoo@gmail.com</a>]
<b>On Behalf Of </b>Richard Smith<br>
<b>Sent:</b> Thursday, April 16, 2015 4:41 PM<br>
<b>To:</b> Li, Charles<br>
<b>Cc:</b> <a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a>; Domizioli, Dario<br>
<b>Subject:</b> Re: Proposed patch (2nd revision) adds TLS Max Alignment diagnostic</span><u></u><u></u></p>
</div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<div>
<p class="MsoNormal">+ if (const unsigned MaxAlign = Context.getTargetInfo().getMaxTLSAlign()) {<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">+ for (auto *I : VD->specific_attrs<AlignedAttr>()) {<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">+ if (I->isAlignmentDependent())<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">+ return;<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Do not return here. You're in the middle of a function that does a bunch of other checks after this point. Perhaps factor out a static hasDependentAlignment helper function?<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">+ }<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">+<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">+ if (VD->getTLSKind()) {<u></u><u></u></p>
</div>
<div>
<div>
<p class="MsoNormal">+ QualType T = VD->getType();<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">+ if (const ReferenceType *RT = T->getAs<ReferenceType>())<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">+ T = Context.getPointerType(RT->getPointeeType());<u></u><u></u></p>
</div>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">This code is redundant. The only place you use T is when you detect whether it's dependent:<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">+ if (!T->isDependentType()) {<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">... which is unaffected by whether it's a pointer or a reference type.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<div>
<p class="MsoNormal">+def err_tls_var_aligned_over_maximum : Error<<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">+ "alignment of thread-local variable %0 is greater than the maximum supported "<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">+ "alignment (%1) for a thread-local variable on this target">;<u></u><u></u></p>
</div>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Maybe include the requested alignment of the variable in this error?<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">+// RUN: %clang_cc1 -triple x86_64-scei-ps4 -fsyntax-only -verify %s<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Please also add something like<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only %s<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">... to verify that we don't emit any diagnostics for the case where we don't have an alignment limit.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<p class="MsoNormal">On Wed, Apr 15, 2015 at 4:04 PM, Li, Charles <<a href="mailto:charles_li@playstation.sony.com" target="_blank">charles_li@playstation.sony.com</a>> wrote:<u></u><u></u></p>
<div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Arial","sans-serif"">Hi Everyone,</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Arial","sans-serif""> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Arial","sans-serif"">We have updated the proposed patch which adds an error diagnostic for when TLS variables exceed maximum TLS alignment.
</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Arial","sans-serif"">This update takes care of the case where alignment is dependent. It also removed the unnecessary getBaseElementType()
call.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Arial","sans-serif""> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Arial","sans-serif"">Sincerely,</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Arial","sans-serif"">Charles Li</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></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""> Li, Charles
<br>
<b>Sent:</b> Friday, April 03, 2015 3:21 PM<br>
<b>To:</b> 'David Majnemer'<br>
<b>Cc:</b> <a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<b>Subject:</b> RE: Proposed patch adds TLS Max Alignment diagnostic</span><u></u><u></u></p>
</div>
</div>
<p class="MsoNormal"> <u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Hi David,</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Oops, we didn’t think of the case where the alignment could be a template parameter.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">I currently don’t have a clue to fix this.
</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Any hint would be greatly welcome.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">On why we used BaseT instead of T.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">I think we originally added</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> QualType BaseT = Context.getBaseElementType(T);</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Just be to conservative as we don’t fully understand how LLVM’s type system when it comes to templates.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">I have re-ran clang replacing BaseT with T and did not notice any regressions.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">So I will take out BaseT for the next revision of the patch.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Cheers,</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Charles Li</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<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""> David Majnemer [<a href="mailto:david.majnemer@gmail.com" target="_blank">mailto:david.majnemer@gmail.com</a>]
<br>
<b>Sent:</b> Wednesday, April 01, 2015 7:08 PM<br>
<b>To:</b> Li, Charles<br>
<b>Cc:</b> <a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<b>Subject:</b> Re: Proposed patch adds TLS Max Alignment diagnostic</span><u></u><u></u></p>
</div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<p class="MsoNormal">Hi Charles,<u></u><u></u></p>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Your patch doesn't handle cases where the alignment is dependent, getDeclAlign doesn't want to be called in such cases:<u></u><u></u></p>
</div>
<div>
<div>
<p class="MsoNormal">template <int N><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">struct S {<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> static int __thread __attribute__((aligned(N))) x;<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">};<u></u><u></u></p>
</div>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Also, why do you use BaseT->isDependentType() instead of T->isDependentType()?<u></u><u></u></p>
</div>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<p class="MsoNormal">On Wed, Apr 1, 2015 at 4:57 PM, Li, Charles <<a href="mailto:charles_li@playstation.sony.com" target="_blank">charles_li@playstation.sony.com</a>> wrote:<u></u><u></u></p>
<div>
<div>
<p class="MsoNormal">Hi Clang developers,<u></u><u></u></p>
<p class="MsoNormal"> <u></u><u></u></p>
<p class="MsoNormal">We here at Sony PlayStation have a proposed patch which adds an error diagnostic for when TLS variables exceed maximum TLS alignment.
<u></u><u></u></p>
<p class="MsoNormal">Please note this patch does not affect normal maximum alignments.
<u></u><u></u></p>
<p class="MsoNormal">This TLS maximum alignment check is currently only turned on for PS4 but could potentially be used for other platforms.<u></u><u></u></p>
<p class="MsoNormal"> <u></u><u></u></p>
<p class="MsoNormal">Sincerely,<u></u><u></u></p>
<p class="MsoNormal">Charles Li<u></u><u></u></p>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
</div>
<p class="MsoNormal" style="margin-bottom:12.0pt"><br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><u></u><u></u></p>
</div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
</div>
</div>
<p class="MsoNormal" style="margin-bottom:12.0pt"><br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><u></u><u></u></p>
</div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
</div>
</div>
</div>
<p class="MsoNormal" style="margin-bottom:12.0pt"><br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><u></u><u></u></p>
</div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
</div>
</div>
</div></div></div>
</div>
</blockquote></div><br></div>