Parse vector bool when stdbool.h and altivec.h are included

[ZarkoCA]

Summary

Currently when including stdbool.h and altivec.h declaration of vector bool leads to
errors due to bool being expanded to '_Bool`. This patch allows the parser
to recognize _Bool.

[nemanjai]

clang/test/Parser/altivec-bool.c

11 ↗

(On Diff #343641)

Do we not want the test case to provide a similar define to what stdbool.h does?

[ZarkoCA]

• update test cases to contain _Bool as well

[ZarkoCA]

clang/test/Parser/altivec-bool.c

11 ↗

(On Diff #343641)

Is using eg adding cases such as: __vector _Bool char bc; what you meant? Added those in just now.

[nemanjai]

This seems fine to me now but I'll defer to front end experts for approval.

[hubert.reinterpretcast]

clang/lib/Parse/ParseDecl.cpp

7347–7349

Minor nit: remove (inconsistently applied) extra parentheses.

clang/lib/Parse/Parser.cpp

516

No need for an extra if block. Just need to figure out which block above this should go in.

[ZarkoCA]

• clean up code per comments
• Add test case for Z targets since that target is also affected

[Everybody0523]

Minor nitpick, but is there a term that encompasses both the Z Vector syntax and altivec? Since the test checks both Z and PPC it's a little odd that the test's filename only references altivec.

[uweigand]

This means the implementation now deviates from the documented vector extension syntax, right? I guess it's strictly an extension of the documented syntax, but that may still lead to incompatibilities with other compilers for the platform. If we want to make such a change, should it be synchronized with e.g. GCC, XL, etc. ?

[ZarkoCA]

In D102064#2751001, @uweigand wrote:

This means the implementation now deviates from the documented vector extension syntax, right? I guess it's strictly an extension of the documented syntax, but that may still lead to incompatibilities with other compilers for the platform. If we want to make such a change, should it be synchronized with e.g. GCC, XL, etc. ?

GCC and XL already accept this syntax on Linux on Power and AIX.

For example this simple test case:

#include <stdbool.h>
#include <altivec.h>

vector bool char bc;

Can compile with GCC 9/10 and XLC 16.1 on Linux on Power. On AIX, GCC 8.3 on AIX and XLC 16.1 can also compile it successfully. Latest main trunk clang throws up an error on those platforms.

From offline conversation it looks like XLC on z/OS can also compile the test case. @Everybody0523 can confirm for sure.

[ZarkoCA]

In D102064#2750930, @Everybody0523 wrote:

Minor nitpick, but is there a term that encompasses both the Z Vector syntax and altivec? Since the test checks both Z and PPC it's a little odd that the test's filename only references altivec.

That's a good point, I can rename the file to altivec-zvector-bool.c?

[uweigand]

In D102064#2751089, @ZarkoCA wrote:

In D102064#2751001, @uweigand wrote:

This means the implementation now deviates from the documented vector extension syntax, right? I guess it's strictly an extension of the documented syntax, but that may still lead to incompatibilities with other compilers for the platform. If we want to make such a change, should it be synchronized with e.g. GCC, XL, etc. ?

GCC and XL already accept this syntax on Linux on Power and AIX.

For example this simple test case:

#include <stdbool.h>
#include <altivec.h>

vector bool char bc;

Can compile with GCC 9/10 and XLC 16.1 on Linux on Power. On AIX, GCC 8.3 on AIX and XLC 16.1 can also compile it successfully. Latest main trunk clang throws up an error on those platforms.

From offline conversation it looks like XLC on z/OS can also compile the test case. @Everybody0523 can confirm for sure.

Interesting. On Z using GCC I currently get this error:

vbool.c:2:1: error: invalid vector type for attribute ‘vector_size’
    2 | vector _Bool x;

But looking at the GCC sources, it seems we actually intended to support this use as well, there's just a bug. Given that, I think I'd be fine with adding this to LLVM -- I'll make sure the GCC bug gets fixed as well.

[ZarkoCA]

In D102064#2751160, @uweigand wrote:

In D102064#2751089, @ZarkoCA wrote:

In D102064#2751001, @uweigand wrote:

This means the implementation now deviates from the documented vector extension syntax, right? I guess it's strictly an extension of the documented syntax, but that may still lead to incompatibilities with other compilers for the platform. If we want to make such a change, should it be synchronized with e.g. GCC, XL, etc. ?

GCC and XL already accept this syntax on Linux on Power and AIX.

For example this simple test case:

#include <stdbool.h>
#include <altivec.h>

vector bool char bc;

Can compile with GCC 9/10 and XLC 16.1 on Linux on Power. On AIX, GCC 8.3 on AIX and XLC 16.1 can also compile it successfully. Latest main trunk clang throws up an error on those platforms.

From offline conversation it looks like XLC on z/OS can also compile the test case. @Everybody0523 can confirm for sure.

Interesting. On Z using GCC I currently get this error:

vbool.c:2:1: error: invalid vector type for attribute ‘vector_size’
    2 | vector _Bool x;

But looking at the GCC sources, it seems we actually intended to support this use as well, there's just a bug. Given that, I think I'd be fine with adding this to LLVM -- I'll make sure the GCC bug gets fixed as well.

Thank you for looking at GCC on Z. That was the only case where I didn't have information from.

[Everybody0523]

xlc on z/OS will compile

vector _Bool char bc;

on any language level high enough to recognize _Bool as a valid keyword.

[ZarkoCA]

• Rename test

[hubert.reinterpretcast]

LGTM with comment.

clang/test/Parser/altivec-zvector-bool.c

21–24

I think having these as declarations of the names used above is beneficial because it confirms that the types formed using the bool spelling are compatible with the same using the _Bool spelling.

[Everybody0523]

Expected Z behavior LGTM

[Andreas-Krebbel]

But looking at the GCC sources, it seems we actually intended to support this use as well, there's just a bug. Given that, I think I'd be fine with adding this to LLVM -- I'll make sure the GCC bug gets fixed as well.

I've fixed the GCC issue now: https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570561.html

clang/include/clang/Parse/Parser.h

class Parser : public CodeCompletionHandler {
 
   /// Contextual keywords for Microsoft extensions.
   IdentifierInfo *Ident__except;
   mutable IdentifierInfo *Ident_sealed;
 
   /// Ident_super - IdentifierInfo for "super", to support fast
   /// comparison.
   IdentifierInfo *Ident_super;
-  /// Ident_vector, Ident_bool - cached IdentifierInfos for "vector" and
-  /// "bool" fast comparison.  Only present if AltiVec or ZVector are enabled.
+  /// Ident_vector, Ident_bool, Ident_Bool - cached IdentifierInfos for "vector"
+  /// and "bool" fast comparison.  Only present if AltiVec or ZVector are
+  /// enabled.
   IdentifierInfo *Ident_vector;
   IdentifierInfo *Ident_bool;
+  IdentifierInfo *Ident_Bool;
   /// Ident_pixel - cached IdentifierInfos for "pixel" fast comparison.
   /// Only present if AltiVec enabled.
   IdentifierInfo *Ident_pixel;
 
   /// Objective-C contextual keywords.
   IdentifierInfo *Ident_instancetype;
 
   /// Identifier for "introduced".
private:
   bool TryAltiVecToken(DeclSpec &DS, SourceLocation Loc,
                        const char *&PrevSpec, unsigned &DiagID,
                        bool &isInvalid) {
     if (!getLangOpts().AltiVec && !getLangOpts().ZVector)
       return false;
 
     if (Tok.getIdentifierInfo() != Ident_vector &&
         Tok.getIdentifierInfo() != Ident_bool &&
+        Tok.getIdentifierInfo() != Ident_Bool &&
         (!getLangOpts().AltiVec || Tok.getIdentifierInfo() != Ident_pixel))
       return false;
 
     return TryAltiVecTokenOutOfLine(DS, Loc, PrevSpec, DiagID, isInvalid);
   }
 
   /// TryAltiVecVectorToken - Check for context-sensitive AltiVec vector
   /// identifier token, replacing it with the non-context-sensitive __vector.

clang/lib/Parse/ParseDecl.cpp

bool Parser::TryAltiVecVectorTokenOutOfLine() {
   case tok::kw_signed:
   case tok::kw_unsigned:
   case tok::kw_void:
   case tok::kw_char:
   case tok::kw_int:
   case tok::kw_float:
   case tok::kw_double:
   case tok::kw_bool:
+  case tok::kw__Bool:
   case tok::kw___bool:
   case tok::kw___pixel:
     Tok.setKind(tok::kw___vector);
     return true;
   case tok::identifier:
     if (Next.getIdentifierInfo() == Ident_pixel) {
       Tok.setKind(tok::kw___vector);
       return true;
     }
-    if (Next.getIdentifierInfo() == Ident_bool) {
+    if (Next.getIdentifierInfo() == Ident_bool ||
+        Next.getIdentifierInfo() == Ident_Bool) {
       Tok.setKind(tok::kw___vector);
       return true;
     }
     return false;
   }
 }
 
 bool Parser::TryAltiVecTokenOutOfLine(DeclSpec &DS, SourceLocation Loc,
                                       const char *&PrevSpec, unsigned &DiagID,
                                       bool &isInvalid) {
   const PrintingPolicy &Policy = Actions.getASTContext().getPrintingPolicy();
   if (Tok.getIdentifierInfo() == Ident_vector) {
     Token Next = NextToken();
     switch (Next.getKind()) {
     case tok::kw_short:
     case tok::kw_long:
     case tok::kw_signed:
     case tok::kw_unsigned:
     case tok::kw_void:
     case tok::kw_char:
     case tok::kw_int:
     case tok::kw_float:
     case tok::kw_double:
     case tok::kw_bool:
+    case tok::kw__Bool:
     case tok::kw___bool:
     case tok::kw___pixel:
       isInvalid = DS.SetTypeAltiVecVector(true, Loc, PrevSpec, DiagID, Policy);
       return true;
     case tok::identifier:
       if (Next.getIdentifierInfo() == Ident_pixel) {
         isInvalid = DS.SetTypeAltiVecVector(true, Loc, PrevSpec, DiagID,Policy);
         return true;
       }
-      if (Next.getIdentifierInfo() == Ident_bool) {
-        isInvalid = DS.SetTypeAltiVecVector(true, Loc, PrevSpec, DiagID,Policy);
+      if (Next.getIdentifierInfo() == Ident_bool ||
+          Next.getIdentifierInfo() == Ident_Bool) {
+        isInvalid =
+            DS.SetTypeAltiVecVector(true, Loc, PrevSpec, DiagID, Policy);
         return true;
       }
       break;
     default:
       break;
     }
   } else if ((Tok.getIdentifierInfo() == Ident_pixel) &&
              DS.isTypeAltiVecVector()) {

clang/lib/Parse/Parser.cpp

void Parser::Initialize() {
   Ident_GNU_final = nullptr;
   Ident_import = nullptr;
   Ident_module = nullptr;
 
   Ident_super = &PP.getIdentifierTable().get("super");
 
   Ident_vector = nullptr;
   Ident_bool = nullptr;
+  Ident_Bool = nullptr;
   Ident_pixel = nullptr;
   if (getLangOpts().AltiVec || getLangOpts().ZVector) {
     Ident_vector = &PP.getIdentifierTable().get("vector");
     Ident_bool = &PP.getIdentifierTable().get("bool");
+    Ident_Bool = &PP.getIdentifierTable().get("_Bool");
   }
   if (getLangOpts().AltiVec)
     Ident_pixel = &PP.getIdentifierTable().get("pixel");
 
   Ident_introduced = nullptr;
   Ident_deprecated = nullptr;
   Ident_obsoleted = nullptr;
   Ident_unavailable = nullptr;
   Ident_strict = nullptr;
   Ident_replacement = nullptr;
 
   Ident_language = Ident_defined_in = Ident_generated_declaration = nullptr;
 

clang/test/Parser/altivec-zvector-bool.c

+// RUN: %clang_cc1 -triple=powerpc64-unknown-linux-gnu \
+// RUN:            -target-feature +altivec -fsyntax-only %s
+// RUN: %clang_cc1 -triple=powerpc64le-unknown-linux-gnu \
+// RUN:            -target-feature +altivec -fsyntax-only %s
+// RUN: %clang_cc1 -triple=powerpc64-ibm-aix-xcoff \
+// RUN:            -target-feature +altivec -fsyntax-only %s
+// RUN: %clang_cc1 -triple=powerpc-ibm-aix-xcoff \
+// RUN:            -target-feature +altivec -fsyntax-only %s
+// RUN: %clang_cc1 -triple=powerpc-unknown-linux-gnu \
+// RUN:            -target-feature +altivec -fsyntax-only %s
+// RUN: %clang_cc1 -triple=s390x-linux-gnu -target-cpu arch11 \
+// RUN:            -fzvector -fsyntax-only %s
+// RUN: %clang_cc1 -triple=s390x-ibm-zos -target-cpu arch11 \
+// RUN:            -fzvector -fsyntax-only %s
+
+__vector bool char bc;
+__vector bool short bsh;
+__vector bool short int bshi;
+__vector bool int bi;
+__vector _Bool char bc;
+__vector _Bool short bsh;
+__vector _Bool short int bshi;
+__vector _Bool int bi;