<div dir="ltr"><div>Sorry all for not initially attaching the patch correctly,</div><div><br></div>Please find the proposed patch attached for your consideration.<div><br></div><div>Regards,</div><div><br></div><div><div>Pierre Gousseau </div><div>SN Systems - Sony Computer Entertainment Group </div></div></div><div class="gmail_extra"><br><div class="gmail_quote">On 13 November 2014 14:14, 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><br></div><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><br></div><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> }</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> }</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>
</blockquote></div><br></div>