<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>On Dec 16, 2009, at 11:45 AM, John Thompson wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div>Nudge nudge...</div>
<div> </div>
<div>I know I need to do an error message checking test too. This is just a start, to avoid too much change at once, and to make sure I'm on the right track. Please review it before it gets too stale.</div>
<div> </div>
<div>-John<br><br></div>
<div class="gmail_quote">On Fri, Dec 11, 2009 at 8:03 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-color: rgb(204, 204, 204); border-left-width: 1px; border-left-style: solid; margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0.8ex; padding-left: 1ex; position: static; z-index: auto; " class="gmail_quote">
<div>Here's a stab a the first stage of adding AltiVec vector support, supporting the AltiVec types __vector, vector, __pixel, pixel, and bool. I've also enclosed a couple of tests for it for the test/Parser directory.</div>
<div> </div>
<div>Some of it might be iffy, but I hope you can help me get it into shape with your feedback.</div>
<div> </div>
<div>I added a couple of flags to the DeclSpec class to help me know when the types are used, TypeAltiVecVector and TypeAltiVecPixel. The AltiVec standard doesn't say much about pixel, other than it is used as "__vector __pixel", and that it represents 8 unsigned shorts. Therefore, instead of making a new TST, I made it more like an alias via the TypeAltiVecPixel flag.</div>
<div> </div>
<div>I also added two corresponding flags to the VectorType class (and the getVectorType function). Currently, these mainly just help with printing, but maybe there will be a need to distinguish down the road. This is kind of an iffy part, because there were a couple of places where I don't know if the vector involved is an AltiVec vector, and the flags are used in the type look up.</div>
<div> </div>
<div>Per the previous conversation, I made __vector and __pixel true keywords if -faltivec is used, and vector and pixel are context-sensitive keywords. The AltiVec standard also specifies that bool is a keyword in C, so I diddled the token flags a bit for that too.</div>
<div> </div>
<div>I put the Token patching code in inline functions, so I hope the performance impact is not too bad.<br clear="all"></div>
<div>There's probably some holes in the testing, i.e. sizeof. But, because I'm piggy-backing on the existing vectors, I'm hoping most things will still just work.</div>
<div> </div>
<div>I'm not sure what the next stage should be, the built-in functions?</div>
<div> </div>
<div>Thanks.</div>
<div> </div>
<div>-John</div></blockquote></div></blockquote><div><br></div><div><br></div><div>+def err_invalid_pixel_decl_spec_combination : Error<</div><div>+ "\"__pixel\" must be preceeded by \"__vector\". '%0' declaration specifier not allowed here">;</div><div><br></div><div>Typo "preceeded".</div><div><br></div><div><div>Index: include/clang/AST/Type.h</div><div>===================================================================</div><div>--- include/clang/AST/Type.h<span class="Apple-tab-span" style="white-space:pre"> </span>(revision 91204)</div><div>+++ include/clang/AST/Type.h<span class="Apple-tab-span" style="white-space:pre"> </span>(working copy)</div><div>@@ -1576,7 +1576,8 @@</div><div> </div><div> /// VectorType - GCC generic vector type. This type is created using</div><div> /// __attribute__((vector_size(n)), where "n" specifies the vector size in</div><div>-/// bytes. Since the constructor takes the number of vector elements, the</div><div>+/// bytes; or from an Altivec __vector or vector declaration.</div><div>+/// Since the constructor takes the number of vector elements, the</div><div> /// client is responsible for converting the size into the number of elements.</div><div> class VectorType : public Type, public llvm::FoldingSetNode {</div><div> protected:</div><div>@@ -1586,13 +1587,21 @@</div><div> /// NumElements - The number of elements in the vector.</div><div> unsigned NumElements;</div><div> </div><div>- VectorType(QualType vecType, unsigned nElements, QualType canonType) :</div><div>+ /// AltiVec - True if this is for an Altivec vector.</div><div>+ bool AltiVec;</div><div>+</div><div>+ /// Pixel - True if this is for an Altivec vector pixel.</div><div>+ bool Pixel;</div><div>+</div><div><br></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><br></div></div><div><div>Index: include/clang/Parse/Parser.h</div><div>===================================================================</div><div>--- include/clang/Parse/Parser.h<span class="Apple-tab-span" style="white-space:pre"> </span>(revision 91204)</div><div>+++ include/clang/Parse/Parser.h<span class="Apple-tab-span" style="white-space:pre"> </span>(working copy)</div><div>@@ -320,6 +320,80 @@</div><div> /// annotated.</div><div> bool TryAnnotateCXXScopeToken(bool EnteringContext = false);</div><div> </div><div>+ /// TryAltiVecToken - Check for context-sensitive AltiVec identifier tokens,</div><div>+ /// replacing them with the non-context-sensitive keywords. This returns</div><div>+ /// true if the token was replaced.</div><div>+ bool TryAltiVecToken(DeclSpec &DS) {</div><div>+ if (getLang().AltiVec) {</div><div>+ if (Tok.getIdentifierInfo()->isStr<7>("vector")) {</div><div><br></div><div>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. </div><div><br></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><br></div><div>+ switch (nextToken.getKind()) {</div><div><div>+ case tok::kw_short:</div><div>+ case tok::kw_long:</div><div>+ case tok::kw_signed:</div><div>+ case tok::kw_unsigned:</div><div>+ case tok::kw_void:</div><div>+ case tok::kw_char:</div><div>+ case tok::kw_int:</div><div>+ case tok::kw_float:</div><div>+ case tok::kw_double:</div><div>+ case tok::kw_bool:</div><div>+ case tok::kw___pixel:</div><div>+ Tok.setKind(tok::kw___vector);</div><div>+ return true;</div><div>+ case tok::identifier:</div><div>+ if (nextToken.getIdentifierInfo()->isStr<6>("pixel")) {</div><div>+ Tok.setKind(tok::kw___vector);</div><div>+ return true;</div><div>+ }</div><div><br></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><br></div><div><div>Index: lib/Sema/SemaType.cpp</div><div>===================================================================</div><div>--- lib/Sema/SemaType.cpp<span class="Apple-tab-span" style="white-space:pre"> </span>(revision 91204)</div><div>+++ lib/Sema/SemaType.cpp<span class="Apple-tab-span" style="white-space:pre"> </span>(working copy)</div><div>@@ -339,6 +339,11 @@</div><div> if (TheSema.getLangOptions().Freestanding)</div><div> TheSema.Diag(DS.getTypeSpecComplexLoc(), diag::ext_freestanding_complex);</div><div> Result = Context.getComplexType(Result);</div><div>+ } else if (DS.isTypeAltiVecVector()) {</div><div>+ unsigned typeSize = static_cast<unsigned>(Context.getTypeSize(Result));</div><div>+ assert(typeSize > 0 && "type size for vector must be greater than 0 bits");</div><div>+ Result = Context.getVectorType(Result, 128/typeSize, true,</div><div>+ DS.isTypeAltiVecPixel());</div><div> }</div><div><br></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><br></div><div><span class="Apple-tab-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">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><br></div><div><div>Index: lib/AST/TypePrinter.cpp</div><div>===================================================================</div><div>--- lib/AST/TypePrinter.cpp<span class="Apple-tab-span" style="white-space:pre"> </span>(revision 91204)</div><div>+++ lib/AST/TypePrinter.cpp<span class="Apple-tab-span" style="white-space:pre"> </span>(working copy)</div><div>@@ -240,15 +240,24 @@</div><div> }</div><div> </div><div> void TypePrinter::PrintVector(const VectorType *T, std::string &S) { </div><div>- // FIXME: We prefer to print the size directly here, but have no way</div><div>- // to get the size of the type.</div><div>- Print(T->getElementType(), S);</div><div>- std::string V = "__attribute__((__vector_size__(";</div><div>- V += llvm::utostr_32(T->getNumElements()); // convert back to bytes.</div><div>- std::string ET;</div><div>- Print(T->getElementType(), ET);</div><div>- V += " * sizeof(" + ET + ")))) ";</div><div>- S = V + S;</div><div>+ if (T->isAltiVec()) {</div><div>+ if (T->isPixel())</div><div>+ S = "vector pixel " + S;</div><div>+ else {</div><div>+ Print(T->getElementType(), S);</div><div>+ S = "vector " + S;</div><div>+ }</div><div><br></div><div>It would probably be better to print with "__vector" and "__pixel", since those keywords are always available.</div></div><div><br></div><div><div>Index: include/clang/AST/ASTContext.h</div><div>===================================================================</div><div>--- include/clang/AST/ASTContext.h<span class="Apple-tab-span" style="white-space:pre"> </span>(revision 91204)</div><div>+++ include/clang/AST/ASTContext.h<span class="Apple-tab-span" style="white-space:pre"> </span>(working copy)</div><div>@@ -498,7 +498,8 @@</div><div> </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 getVectorType(QualType VectorType, unsigned NumElts);</div><div>+ QualType getVectorType(QualType VectorType, unsigned NumElts,</div><div>+ bool AltiVec, bool IsPixel);</div><div><br></div><div>Index: lib/AST/ASTContext.cpp</div></div><div><div>===================================================================</div><div>--- lib/AST/ASTContext.cpp<span class="Apple-tab-span" style="white-space:pre"> </span>(revision 91204)</div><div>+++ lib/AST/ASTContext.cpp<span class="Apple-tab-span" style="white-space:pre"> </span>(working copy)</div><div>@@ -1590,7 +1590,8 @@</div><div> </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>@@ -1598,7 +1599,8 @@</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>@@ -1607,14 +1609,15 @@</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>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><br></div><div><div>Index: lib/Parse/ParseDecl.cpp</div><div>===================================================================</div><div>--- lib/Parse/ParseDecl.cpp<span class="Apple-tab-span" style="white-space:pre"> </span>(revision 91204)</div><div>+++ lib/Parse/ParseDecl.cpp<span class="Apple-tab-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">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><br></div><div><div>+bool DeclSpec::SetTypeAltiVecPixel(bool isAltiVecPixel, SourceLocation Loc,</div><div>+ const char *&PrevSpec, unsigned &DiagID) {</div><div>+ if (!TypeAltiVecVector || (TypeSpecType != TST_unspecified)) {</div><div>+ PrevSpec = DeclSpec::getSpecifierName((TST) TypeSpecType);</div><div>+ DiagID = diag::err_invalid_pixel_decl_spec_combination;</div><div>+ return true;</div><div>+ }</div><div>+ TypeSpecType = TST_int;</div><div>+ TypeSpecSign = TSS_unsigned;</div><div>+ TypeSpecWidth = TSW_short;</div><div>+ TypeAltiVecPixel = isAltiVecPixel;</div><div>+ TSTLoc = Loc;</div><div>+ return false;</div><div>+}</div><div>+</div><div><br></div><div>pixel is an unsigned short? Weird.</div><div><br></div><div>Thanks for working on this!</div><div><br></div></div></div></div></div><div><span class="Apple-tab-span" style="white-space:pre"> </span>- Doug</div></div></div></div></body></html>