<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">Hi John,<div><br></div><div>I missed this patch in the post-holiday flood; feel free to kick me if more than a few days goes by and I haven't commented on a patch..</div><div><br></div><div><div>Index: include/clang/AST/Type.h</div><div><br></div><div>===================================================================</div><div>--- include/clang/AST/Type.h<span class="Apple-tab-span" style="white-space:pre"> </span>(revision 94011)</div><div>+++ include/clang/AST/Type.h<span class="Apple-tab-span" style="white-space:pre"> </span>(working copy)</div><div>@@ -1602,14 +1611,28 @@</div><div><br></div><div> bool isSugared() const { return false; }</div><div> QualType desugar() const { return QualType(this, 0); }</div><div> </div><div>+ bool isAltiVec() const { return AltiVec; }</div><div>+ void setAltiVec(bool isAltiVec) { AltiVec = isAltiVec; }</div><div>+ </div><div>+ bool isPixel() const { return Pixel; }</div><div>+ void setPixel(bool isPixel) { Pixel = isPixel; }</div><div><br></div><div>Type classes shouldn't have setters in them, because Types are immutable. The AltiVec and Pixel bits should only be passed in via the constructor arguments.</div><div><br></div><div><div>+ bool isImplicitCastable(const VectorType *ToType) const {</div><div>+ return((ToType->ElementType == ElementType) &&</div><div>+ (ToType->NumElements == NumElements)); }</div><div><br></div><div>We generally don't have this kind of predicate within the Type hierarchy. Convertibility information is generally captured in the type-compatibility routines in ASTContext (for C) and in the implicit conversions in Sema (for C++). And, this function isn't actually correct: casting should be looking at the canonical types to determine compatibility/equivalence and cv-qualifiers need to be taken into account.</div><div><br></div><div><div>Index: lib/AST/ASTContext.cpp</div><div>===================================================================</div><div>--- lib/AST/ASTContext.cpp<span class="Apple-tab-span" style="white-space:pre"> </span>(revision 94011)</div><div>+++ lib/AST/ASTContext.cpp<span class="Apple-tab-span" style="white-space:pre"> </span>(working copy)</div><div>@@ -1596,7 +1596,8 @@</div><div><br></div><div> /// getVectorType - Return the unique reference to a vector type of</div><div> /// the specified element type and size. VectorType must be a built-in type.</div><div>-QualType ASTContext::getVectorType(QualType vecType, unsigned NumElts) {</div><div>+QualType ASTContext::getVectorType(QualType vecType, unsigned NumElts,</div><div>+ bool IsAltiVec, bool IsPixel) {</div><div> BuiltinType *baseType;</div><div> </div><div> baseType = dyn_cast<BuiltinType>(getCanonicalType(vecType).getTypePtr());</div><div>@@ -1604,7 +1605,8 @@</div><div><br></div><div> </div><div> // Check if we've already instantiated a vector of this type.</div><div> llvm::FoldingSetNodeID ID;</div><div>- VectorType::Profile(ID, vecType, NumElts, Type::Vector);</div><div>+ VectorType::Profile(ID, vecType, NumElts, Type::Vector,</div><div>+ IsAltiVec, IsPixel);</div><div> void *InsertPos = 0;</div><div> if (VectorType *VTP = VectorTypes.FindNodeOrInsertPos(ID, InsertPos))</div><div> return QualType(VTP, 0);</div><div>@@ -1613,14 +1615,15 @@</div><div><br></div><div> // so fill in the canonical type field.</div><div> QualType Canonical;</div><div> if (!vecType.isCanonical()) {</div><div>- Canonical = getVectorType(getCanonicalType(vecType), NumElts);</div><div>+ Canonical = getVectorType(getCanonicalType(vecType),</div><div>+ NumElts, IsAltiVec, IsPixel);</div></div><div><br></div><div>Here, we're making the AltiVec and Pixel bits part of the canonical type, which means that, for example, the two typedefs below name different types:</div><div><br></div><div><div><span class="Apple-tab-span" style="white-space:pre"> </span>typedef vector unsigned int altivec_vector;</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>typedef __attribute__((vector_size(16))) unsigned int gcc_vector;</div><div><br></div><div>However, GCC seems to be treating the types as identical. Using those typedefs with the following program:</div><div><br></div><div><div>template<typename T, typename U></div><div>struct is_same {</div><div> static const bool value = false;</div><div>};</div><div><br></div><div>template<typename T></div><div>struct is_same<T, T> {</div><div> static const bool value = true;</div><div>};</div><div><br></div><div>int array0[is_same<altivec_vector, gcc_vector>::value? 1 : -1];</div><div><br></div><div>altivec_vector ***p1;</div><div>gcc_vector ***p2 = p1;</div><div>altivec_vector ***p3 = p2;</div><div><br></div><div>void f0(altivec_vector) { }</div><div>void f0(gcc_vector) { }</div><div><br></div><div>we get the results we would expect if altivec_vector and gcc_vector were just typedefs of the same type, meaning:</div><div><br></div><div><span class="Apple-tab-span" style="white-space:pre"> </span>- is_same<altivec_vector, gcc_vector>::value is true, indicating that GCC considers these types equivalent in C++</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>- the pointer initializations work</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>- the second definition of f0() is flagged as an error, since f0(altivec_vector) is already defined</div><div><br></div><div>This tells me that the AltiVec vector types are identical to the GCC vector types, which actually simplifies things considerably. It makes isImplicitCastable() unnecessary, since you don't need to cast between two identical types, and a lot of the other interactions between GCC and AltiVec vector types just work: there's no need to change the type-compatibility or type-conversion rules.</div><div><br></div><div>From a practical standpoint, it means that this:</div><div><br></div><div><div> // so fill in the canonical type field.</div><div> QualType Canonical;</div><div> if (!vecType.isCanonical()) {</div><div>- Canonical = getVectorType(getCanonicalType(vecType), NumElts);</div><div>+ Canonical = getVectorType(getCanonicalType(vecType),</div><div>+ NumElts, IsAltiVec, IsPixel);</div><div><br></div><div>should turn into:</div><div><br></div><div><div> // so fill in the canonical type field.</div><div> QualType Canonical;</div><div> if (!vecType.isCanonical() || IsAltiVec || IsPixel) {</div><div>- Canonical = getVectorType(getCanonicalType(vecType), NumElts);</div><div>+ Canonical = getVectorType(getCanonicalType(vecType),</div><div>+ NumElts, false, false);</div></div></div><div><br></div></div></div><div>so that the canonical type of an AltiVec vector type is the same as the equivalent GCC vector type. However, we still have the AltiVec and Pixel flags in the VectorType representation, so that we can print the type as the user wrote it.</div><div><br></div><div><div>Index: include/clang/Parse/DeclSpec.h</div><div>===================================================================</div><div>--- include/clang/Parse/DeclSpec.h<span class="Apple-tab-span" style="white-space:pre"> </span>(revision 94011)</div><div>+++ include/clang/Parse/DeclSpec.h<span class="Apple-tab-span" style="white-space:pre"> </span>(working copy)</div><div><div>@@ -153,6 +154,8 @@</div><div><br></div><div> /*TSC*/unsigned TypeSpecComplex : 2;</div><div> /*TSS*/unsigned TypeSpecSign : 2;</div><div> /*TST*/unsigned TypeSpecType : 5;</div><div>+ bool TypeAltiVecVector : 1; // At present, these two flags</div><div>+ bool TypeAltiVecPixel : 1; // only affect display output.</div><div> bool TypeSpecOwned : 1;</div><div><br></div><div>If this is true, Parse/DeclSpec.h isn't the right place to put the comment. The parser is only responsible for reporting what it parsed to semantic analysis; the AST and Sema libraries are responsible for describing and coping with what the parser parsed. I think this comment belongs on the VectorType itself.</div><div><br></div><div><div>Index: lib/Sema/SemaOverload.cpp</div><div>===================================================================</div><div>--- lib/Sema/SemaOverload.cpp<span class="Apple-tab-span" style="white-space:pre"> </span>(revision 94011)</div><div>+++ lib/Sema/SemaOverload.cpp<span class="Apple-tab-span" style="white-space:pre"> </span>(working copy)</div><div><br></div><div>@@ -708,6 +708,12 @@</div><div><br></div><div> } else if (IsNoReturnConversion(Context, FromType, ToType, FromType)) {</div><div> // Treat a conversion that strips "noreturn" as an identity conversion.</div><div> SCS.Second = ICK_NoReturn_Adjustment;</div><div>+ } else if (FromType->isVectorType() && ToType->isVectorType() &&</div><div>+ FromType->getAs<VectorType>()->isImplicitCastable(</div><div>+ ToType->getAs<VectorType>())) {</div><div>+ // Vector conversions</div><div>+ SCS.Second = ICK_Identity;</div><div>+ FromType = ToType.getUnqualifiedType();</div><div> } else {</div><div> // No second conversion required.</div><div> SCS.Second = ICK_Identity;</div><div><br></div></div><div>This shouldn't be necessary when the canonical-types change above.</div><div><br></div><div>With the canonical-type changes above, this is ready to go in. Thanks!</div><div><br></div></div></div></div></div><div><span class="Apple-tab-span" style="white-space:pre"> </span>- Doug</div><div><br><div><div>On Jan 20, 2010, at 9:21 PM, John Thompson wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">Sorry, I forgot to attach the updated patch.<br><br>
<div class="gmail_quote">On Wed, Jan 20, 2010 at 4:11 PM, John Thompson <span dir="ltr"><<a href="mailto:john.thompson.jtsoftware@gmail.com">john.thompson.jtsoftware@gmail.com</a>></span> wrote:<br>
<blockquote style="BORDER-LEFT: #ccc 1px solid; MARGIN: 0px 0px 0px 0.8ex; PADDING-LEFT: 1ex" class="gmail_quote">Hi, I haven't heard back about this patch regarding a first step towards AltiVec support. Is it okay to check-in?
<div>
<div></div>
<div class="h5"><br><br>
<div class="gmail_quote">On Tue, Jan 5, 2010 at 9:05 PM, John Thompson <span dir="ltr"><<a href="mailto:john.thompson.jtsoftware@gmail.com" target="_blank">john.thompson.jtsoftware@gmail.com</a>></span> wrote:<br>
<blockquote style="BORDER-LEFT: #ccc 1px solid; MARGIN: 0px 0px 0px 0.8ex; PADDING-LEFT: 1ex" class="gmail_quote">
<div>
<div>Sorry, I forgot to include the two updated altivec tests, plus update the tree before making the patch. Please disregard the altivec1a.patch and look at this one instead.</div>
<div> </div>
<div>Thanks.</div>
<div> </div>
<div>-John<br><br></div></div>
<div>
<div></div>
<div>
<div class="gmail_quote">On Tue, Jan 5, 2010 at 6:33 PM, John Thompson <span dir="ltr"><<a href="mailto:john.thompson.jtsoftware@gmail.com" target="_blank">john.thompson.jtsoftware@gmail.com</a>></span> wrote:<br>
<blockquote style="BORDER-LEFT: #ccc 1px solid; MARGIN: 0px 0px 0px 0.8ex; PADDING-LEFT: 1ex" class="gmail_quote">
<div>Thanks, Doug.</div>
<div> </div>
<div>
<blockquote style="BORDER-LEFT: #ccc 1px solid; MARGIN: 0px 0px 0px 0.8ex; PADDING-LEFT: 1ex" class="gmail_quote">Typo "preceeded".<br></blockquote></div>
<div>Fixed.</div>
<div><br> </div>
<div class="gmail_quote">
<div>
<blockquote style="BORDER-LEFT: #ccc 1px solid; MARGIN: 0px 0px 0px 0.8ex; PADDING-LEFT: 1ex" class="gmail_quote">
<div style="WORD-WRAP: break-word">
<div>
<div>
<div>
<div>I think this is right, but I want to check: are AltiVec vectors more like GCC's vectors (handled by VectorType) or more like OpenCL's vectors (handled by ExtVectorType)? OpenCL vectors are mainly GCC's vectors + the swizzling syntax like "myvec.xy".</div>
<div> </div></div></div></div></div></blockquote>
<div> </div></div>
<div>I checked the Altivec standard, and don't see anything like the swizzling syntax, so I think they're more like the GCC vectors.</div>
<div>
<div><br></div>
<blockquote style="BORDER-LEFT: #ccc 1px solid; MARGIN: 0px 0px 0px 0.8ex; PADDING-LEFT: 1ex" class="gmail_quote">It would be more efficient just to keep the IdentiferInfo * for the identifiers "vector" and "pixel" in the parser (only when AltiVec is enabled, of course), then do a pointer comparison rather than a string comparison. </blockquote>
</div></div>
<div class="gmail_quote"> </div>
<div class="gmail_quote">Good idea. I've fixed this.</div>
<div>
<div class="gmail_quote"> </div>
<div class="gmail_quote"><br></div>
<blockquote style="BORDER-LEFT: #ccc 1px solid; MARGIN: 0px 0px 0px 0.8ex; PADDING-LEFT: 1ex" class="gmail_quote">
<div style="WORD-WRAP: break-word">
<div>
<div>
<div>
<div>+ const Token &nextToken = NextToken();</div>
<div><br></div>
<div>Tokens are small; it's best to just copy them, e.g.,</div>
<div><br></div>
<div> Token nextToken = NextToken();</div>
<div> </div></div></div></div></div></blockquote>
<div> </div></div>
<div>Got it.</div>
<div>
<div> </div>
<blockquote style="BORDER-LEFT: #ccc 1px solid; MARGIN: 0px 0px 0px 0.8ex; PADDING-LEFT: 1ex" class="gmail_quote">
<div style="WORD-WRAP: break-word">
<div>
<div>
<div>
<div>By changing the current token, we lose track of whether we saw "vector" or "__vector"; I guess it doesn't really matter. Still, wouldn't it be easier just to update the DeclSpec at this point?</div>
</div></div></div></div></blockquote>
<div> </div></div>
<div>Yep.</div>
<div>
<div><br> </div>
<blockquote style="BORDER-LEFT: #ccc 1px solid; MARGIN: 0px 0px 0px 0.8ex; PADDING-LEFT: 1ex" class="gmail_quote">
<div style="WORD-WRAP: break-word">
<div>
<div>
<div>
<div>
<div>At some point, we'll need more semantic checking here, e.g., to ignore "long long" and complain about the deprecation of "long", if I'm to believe</div></div></div></div></div></div></blockquote>
<div> </div></div>
<div>Yep. I put in a warning for use of "long", and an error for use of "double".</div>
<div>
<div> </div>
<blockquote style="BORDER-LEFT: #ccc 1px solid; MARGIN: 0px 0px 0px 0.8ex; PADDING-LEFT: 1ex" class="gmail_quote">
<div style="WORD-WRAP: break-word">
<div>
<div>
<div>
<div>
<div><span style="WHITE-SPACE: pre"><a href="http://publib.boulder.ibm.com/infocenter/macxhelp/v6v81/index.jsp?topic=/com.ibm.vacpp6m.doc/language/ref/clrc03altivec_types.htm" target="_blank">http://publib.boulder.ibm.com/infocenter/macxhelp/v6v81/index.jsp?topic=/com.ibm.vacpp6m.doc/language/ref/clrc03altivec_types.htm</a></span></div>
<div><br></div>
<div>Or perhaps there's some actual AltiVec specification I could use to help understand this stuff better?</div></div></div></div></div></div></blockquote>
<div> </div></div>
<div>Here's the one I've been using: <a href="http://www.freescale.com/files/32bit/doc/ref_manual/ALTIVECPIM.pdf" target="_blank">http://www.freescale.com/files/32bit/doc/ref_manual/ALTIVECPIM.pdf</a></div>
<div>
<div> </div>
<blockquote style="BORDER-LEFT: #ccc 1px solid; MARGIN: 0px 0px 0px 0.8ex; PADDING-LEFT: 1ex" class="gmail_quote">
<div style="WORD-WRAP: break-word">
<div>
<div>
<div>
<div>
<div>
<div>It would probably be better to print with "__vector" and "__pixel", since those keywords are always available.</div></div></div></div></div></div></div></blockquote>
<div> </div></div>
<div>Got it.</div>
<div>
<div> </div>
<blockquote style="BORDER-LEFT: #ccc 1px solid; MARGIN: 0px 0px 0px 0.8ex; PADDING-LEFT: 1ex" class="gmail_quote">
<div style="WORD-WRAP: break-word">
<div>
<div>
<div>
<div>
<div>
<div>This makes it so that AltiVec vectors are completely distinct types from equivalently-sized vectors (as declared with __attribute__((vector_size(blah))). If we want to be able to cast between AltiVec vectors and GCC-style vectors, we'll need to add type-compatibility logic (for C) and type-conversion logic (for C++). I don't know which way is correct... how does GCC handle compatibility between the various kinds of vectors? We'll need to follow its lead.</div>
</div></div></div></div></div></div></blockquote>
<div> </div></div>
<div>I took a crack at this in SemaOverload.cpp.</div>
<div>
<div> </div>
<blockquote style="BORDER-LEFT: #ccc 1px solid; MARGIN: 0px 0px 0px 0.8ex; PADDING-LEFT: 1ex" class="gmail_quote">
<div style="WORD-WRAP: break-word">
<div>
<div>
<div>
<div>
<div>
<div><br></div>
<div>
<div>Index: lib/Parse/ParseDecl.cpp</div>
<div>===================================================================</div>
<div>--- lib/Parse/ParseDecl.cpp<span style="WHITE-SPACE: pre"> </span>(revision 91204)</div>
<div>+++ lib/Parse/ParseDecl.cpp<span style="WHITE-SPACE: pre"> </span>(working copy)</div>
<div>@@ -393,6 +393,12 @@</div>
<div> }</div>
<div> </div>
<div> if (AllowFunctionDefinitions && D.isFunctionDeclarator()) {</div>
<div>+ // If attributes are present, parse them now.</div>
<div>+ if (<a href="http://tok.is/" target="_blank">Tok.is</a>(tok::kw___attribute)) {</div>
<div>+ SourceLocation Loc;</div>
<div>+ AttributeList *AttrList = ParseGNUAttributes(&Loc);</div>
<div>+ D.AddAttributes(AttrList, Loc);</div>
<div>+ }</div>
<div><br></div>
<div>This seems totally unrelated to AltiVec vectors?</div></div></div></div></div></div></div></div></blockquote>
<div> </div></div>
<div>Sorry. Some code from my attribute position fix that hasn't gotten through yet.</div>
<div>I pulled it for now.</div>
<div>
<div> </div>
<blockquote style="BORDER-LEFT: #ccc 1px solid; MARGIN: 0px 0px 0px 0.8ex; PADDING-LEFT: 1ex" class="gmail_quote">
<div style="WORD-WRAP: break-word">
<div>
<div>
<div>
<div>
<div>
<div>
<div>pixel is an unsigned short? Weird.</div></div></div></div></div></div></div></div></blockquote>
<div> </div></div>
<div>Yeah, totally weird.</div>
<div> </div>
<div>I've enclosed an updated patch.</div>
<div> </div><font color="#888888">
<div>-John</div>
<div> </div></font></blockquote></div><br><br clear="all"><br></div></div>
<div>
<div></div>
<div>-- <br>John Thompson<br><a href="mailto:John.Thompson.JTSoftware@gmail.com" target="_blank">John.Thompson.JTSoftware@gmail.com</a><br><br></div></div></blockquote></div><br><br clear="all"><br>-- <br>John Thompson<br>
<a href="mailto:John.Thompson.JTSoftware@gmail.com" target="_blank">John.Thompson.JTSoftware@gmail.com</a><br><br></div></div></blockquote></div><br><br clear="all"><br>-- <br>John Thompson<br><a href="mailto:John.Thompson.JTSoftware@gmail.com">John.Thompson.JTSoftware@gmail.com</a><br>
<br>
<span><altivec1c.patch></span>_______________________________________________<br>cfe-commits mailing list<br><a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits<br></blockquote></div><br></div></body></html>