<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Nov 13, 2014 at 6:14 AM, pierre gousseau <span dir="ltr"><<a href="mailto:pierregousseau14@gmail.com" target="_blank">pierregousseau14@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>Dear All,</div><div><br></div><div>I would like to propose a fix to an ABI issue found with the new ABI test suite.</div><div>This is my first patch for clang so please bear with me :-) </div></div></blockquote><div><br></div><div>Thanks for the investigation and the patch!</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>The test case is:</div><div><br></div><div>clang version 3.6.0 (221593) </div><div>clang -cc1 -triple i386-pc-linux -S -emit-llvm </div><div>---- </div><div>#ifdef BAD </div><div>template <class T> void g3(char (&buffer)[sizeof(T() + 5)]) {} </div><div>void call_g3() { </div><div>char buffer[sizeof(int)]; </div><div>g3<int>(buffer); </div><div>} </div><div>#endif </div><div>template <class T> void g4(char (&buffer)[sizeof(T() + 5L)]) {} </div><div>void call_g4() { </div><div>char buffer[sizeof(long int)]; </div><div>g4<long int>(buffer); </div><div>} </div><div>----</div><div><br></div><div>In the test case above the mangled name for g4 is different depending on the presence of g3. </div><div><br></div><div>When g3 is not defined, g4 mangled name is : _Z2g4IlEvRAszplcvT__ELl5E_c</div><div>When g3 is defined, g4 mangled name is : _Z2g4IlEvRAszplcvT__ELi5E_c</div><div><br></div><div>The issue seem to be caused by the AST mechanism that decides when existing nodes can be reused to deduce the type of X in the expression "char (&)[X]". </div><div>Where "X" is a type dependent expression (eg T() + 5L).</div><div>The mechanism seems to only take the size of the integer literal "5" to discriminate between "5" and "5L".</div><div>So the patch adds the "builtin kind" to the list of attributes to consider, when discriminating between AST nodes.</div><div><br></div><div>While the anomaly was initially spotted between "int" and "long int" on X86, the investigation led to see that the problem also existed for targets where "double" and "long double" are the same size so the triple "mips-none-none" was chosen for the regression test.</div><div><br></div><div>Please let me know if this is an acceptable change. </div></div></blockquote><div><br></div><div>Yes, this is the right approach.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>Regards, </div><div><br></div><div>Pierre Gousseau </div><div>SN Systems - Sony Computer Entertainment Group </div><div><br></div><div>---</div><div>Index: svn/upstream/tools/clang/lib/AST/StmtProfile.cpp</div><div>===================================================================</div><div>--- svn/upstream/tools/clang/lib/AST/StmtProfile.cpp<span style="white-space:pre-wrap"> </span>(revision 221593)</div><div>+++ svn/upstream/tools/clang/lib/AST/StmtProfile.cpp<span style="white-space:pre-wrap"> </span>(working copy)</div><div>@@ -501,6 +501,10 @@</div><div> void StmtProfiler::VisitIntegerLiteral(const IntegerLiteral *S) {</div><div> VisitExpr(S);</div><div> S->getValue().Profile(ID);</div><div>+ SplitQualType splitType = S->getType().split();</div><div>+ const BuiltinType * builtinType = splitType.Ty->getAs<BuiltinType>();</div><div>+ if (builtinType)</div><div>+ ID.AddInteger(builtinType->getKind());</div></div></blockquote><div><br></div><div>You can use</div><div><br></div><div> ID.AddInteger(S->getType()->castAs<BuiltinType>()->getKind());</div><div><br></div><div>here, because the integer literal is guaranteed to be of a builtin type. (And no need to split the type manually; QualType's operator-> does the right thing here.)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div> }</div><div> </div><div> void StmtProfiler::VisitCharacterLiteral(const CharacterLiteral *S) {</div><div>@@ -513,6 +517,10 @@</div><div> VisitExpr(S);</div><div> S->getValue().Profile(ID);</div><div> ID.AddBoolean(S->isExact());</div><div>+ SplitQualType splitType = S->getType().split();</div><div>+ const BuiltinType * builtinType = splitType.Ty->getAs<BuiltinType>();</div><div>+ if (builtinType)</div><div>+ ID.AddInteger(builtinType->getKind());</div></div></blockquote><div><br></div><div>Likewise here.</div><div><br></div><div>Do you have an SVN account, or do you need someone to commit this for you?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div> }</div><div> </div><div> void StmtProfiler::VisitImaginaryLiteral(const ImaginaryLiteral *S) {</div><div>Index: svn/upstream/tools/clang/test/CodeGen/mips-wrong-template-mangling.cpp</div><div>===================================================================</div><div>--- svn/upstream/tools/clang/test/CodeGen/mips-wrong-template-mangling.cpp<span style="white-space:pre-wrap"> </span>(revision 0)</div><div>+++ svn/upstream/tools/clang/test/CodeGen/mips-wrong-template-mangling.cpp<span style="white-space:pre-wrap"> </span>(working copy)</div><div>@@ -0,0 +1,38 @@</div><div>+// This checks that the mangling of g4 and g6 is not dependent on the presence</div><div>+// of g3 and g5, the bug was due to the AST nodes for the array size expressions</div><div>+// being wrongly shared between g3/g4 and g5/g6 on targets where the single</div><div>+// "long" keyword has no effects</div><div>+// RUN: %clang_cc1 -triple mips-none-none -emit-llvm -o - %s | FileCheck %s</div><div>+// RUN: %clang_cc1 -triple mips-none-none -DBAD -emit-llvm -o - %s | FileCheck -check-prefix=CHECK-BAD %s</div><div>+</div><div>+#ifdef BAD</div><div>+template <class T> void g3(char (&buffer)[sizeof(T() + 5.0)]) {}</div><div>+void call_g3() {</div><div>+ char buffer[sizeof(double)];</div><div>+ // CHECK-BAD: _Z2g3IdEvRAszplcvT__ELd4014000000000000E_c</div><div>+ g3<double>(buffer);</div><div>+}</div><div>+#endif</div><div>+template <class T> void g4(char (&buffer)[sizeof(T() + 5.0L)]) {}</div><div>+void call_g4() {</div><div>+ char buffer[sizeof(long double)];</div><div>+ // CHECK: _Z2g4IeEvRAszplcvT__ELe4014000000000000E_c</div><div>+ // CHECK-BAD: _Z2g4IeEvRAszplcvT__ELe4014000000000000E_c</div><div>+ g4<long double>(buffer);</div><div>+}</div><div>+</div><div>+#ifdef BAD</div><div>+template <class T> void g5(char (&buffer)[sizeof(T() + 5)]) {}</div><div>+void call_g5() {</div><div>+ char buffer[sizeof(int)];</div><div>+ // CHECK-BAD: _Z2g5IiEvRAszplcvT__ELi5E_c </div><div>+ g5<int>(buffer);</div><div>+}</div><div>+#endif</div><div>+template <class T> void g6(char (&buffer)[sizeof(T() + 5L)]) {}</div><div>+void call_g6() {</div><div>+ char buffer[sizeof(long int)];</div><div>+ // CHECK: _Z2g6IlEvRAszplcvT__ELl5E_c</div><div>+ // CHECK-BAD: _Z2g6IlEvRAszplcvT__ELl5E_c</div><div>+ g6<long int>(buffer);</div><div>+}</div><div>---</div></div>
<br>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">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><br>
<br></blockquote></div><br></div></div>