[Sema] Allow creating types with multiple of the same addrspace.

[ebevhan]

Summary

The comment with the OpenCL clause about this clearly
says: "No type shall be qualified by qualifiers for
two or more different address spaces."

This must mean that two or more qualifiers for the
_same_ address space is allowed. However, it is
likely unintended by the programmer, so emit a
warning.

For dependent address space types, reject them like
before since we cannot know what the address space
will be.

[bader]

test/Sema/address_spaces.c

17

I think it might be valuable to give a warning or remark to user.
Using the same address space qualifier multiple times is not something OpenCL C developers are supposed to do.

[ebevhan]

test/Sema/address_spaces.c

17

The test is obviously a bit contrived, but it could happen by mistake, or as a result of some typedef or macro combination. It also cannot go wrong, so there's no harm in it happening.

I see your point, though. A warning feels like a bit much, so I'm not sure what else to use. A note?

[Anastasia]

test/Sema/address_spaces.c

17

Just checked for const qualifier we get a warning:

warning: duplicate 'const' declaration specifier [-Wduplicate-decl-specifier]

We could do the same... not sure if we could try to share the diagnostic as well.

[ebevhan]

test/Sema/address_spaces.c

17

I have a patch ready that adds a warning in the same warning group and uses that. I'm not sure about reusing the other one since address spaces don't have to be declaration specifiers.

The warning is 'multiple identical address spaces specified for type', similar to the error. Is that acceptable?

[bader]

test/Sema/address_spaces.c

17

Sounds good to me. Could you share your patch, please?

[ebevhan]

Added a warning for identical address spaces.

[Anastasia]

LGTM! Thanks!

[ebevhan]

Thanks! I do not have commit access, so it would be great if someone could commit this.

include/clang/Basic/DiagnosticSemaKinds.td

 def err_ext_vector_component_name_illegal : Error<
   "illegal vector component name '%0'">;
 def err_attribute_address_space_negative : Error<
   "address space is negative">;
 def err_attribute_address_space_too_high : Error<
   "address space is larger than the maximum supported (%0)">;
 def err_attribute_address_multiple_qualifiers : Error<
   "multiple address spaces specified for type">;
+def warn_attribute_address_multiple_identical_qualifiers : Warning<
+  "multiple identical address spaces specified for type">,
+  InGroup<DuplicateDeclSpecifier>;
 def err_attribute_address_function_type : Error<
   "function type may not be qualified with an address space">;
 def err_as_qualified_auto_decl : Error<
   "automatic variable qualified with an%select{| invalid}0 address space">;
 def err_arg_with_address_space : Error<
   "parameter may not be qualified with an address space">;
 def err_field_with_address_space : Error<
   "field may not be qualified with an address space">;

lib/Sema/SemaType.cpp

 /// BuildAddressSpaceAttr - Builds a DependentAddressSpaceType if an expression 
 /// is uninstantiated. If instantiated it will apply the appropriate address space 
 /// to the type. This function allows dependent template variables to be used in
 /// conjunction with the address_space attribute  
 QualType Sema::BuildAddressSpaceAttr(QualType &T, Expr *AddrSpace,
                                      SourceLocation AttrLoc) {
   if (!AddrSpace->isValueDependent()) { 
 
-    // If this type is already address space qualified, reject it.
-    // ISO/IEC TR 18037 S5.3 (amending C99 6.7.3): "No type shall be qualified
-    // by qualifiers for two or more different address spaces."
-    if (T.getAddressSpace() != LangAS::Default) {
-      Diag(AttrLoc, diag::err_attribute_address_multiple_qualifiers);
-      return QualType();
-    }
-
     llvm::APSInt addrSpace(32);
     if (!AddrSpace->isIntegerConstantExpr(addrSpace, Context)) {
       Diag(AttrLoc, diag::err_attribute_argument_type)
           << "'address_space'" << AANT_ArgumentIntegerConstant
           << AddrSpace->getSourceRange();
       return QualType();
     }
 
if (addrSpace > max) {
       Diag(AttrLoc, diag::err_attribute_address_space_too_high)
           << (unsigned)max.getZExtValue() << AddrSpace->getSourceRange();
       return QualType();
     }
 
     LangAS ASIdx =
         getLangASFromTargetAS(static_cast<unsigned>(addrSpace.getZExtValue()));
 
+    // If this type is already address space qualified with a different
+    // address space, reject it.
+    // ISO/IEC TR 18037 S5.3 (amending C99 6.7.3): "No type shall be qualified
+    // by qualifiers for two or more different address spaces."
+    if (T.getAddressSpace() != LangAS::Default) {
+      if (T.getAddressSpace() != ASIdx) {
+        Diag(AttrLoc, diag::err_attribute_address_multiple_qualifiers);
+        return QualType();
+      } else
+        // Emit a warning if they are identical; it's likely unintended.
+        Diag(AttrLoc,
+             diag::warn_attribute_address_multiple_identical_qualifiers);
+    }
+
     return Context.getAddrSpaceQualType(T, ASIdx);
   }
 
   // A check with similar intentions as checking if a type already has an
   // address space except for on a dependent types, basically if the 
   // current type is already a DependentAddressSpaceType then its already 
   // lined up to have another address space on it and we can't have
   // multiple address spaces on the one pointer indirection
   if (T->getAs<DependentAddressSpaceType>()) {
     Diag(AttrLoc, diag::err_attribute_address_multiple_qualifiers);
     return QualType();
   }
 
   return Context.getDependentAddressSpaceType(T, AddrSpace, AttrLoc);
 }
 
 /// HandleAddressSpaceTypeAttribute - Process an address_space attribute on the
 /// specified type.  The attribute contains 1 argument, the id of the address
 /// space for the type.
 static void HandleAddressSpaceTypeAttribute(QualType &Type,
                                             const AttributeList &Attr, Sema &S){
-  // If this type is already address space qualified, reject it.
-  // ISO/IEC TR 18037 S5.3 (amending C99 6.7.3): "No type shall be qualified by
-  // qualifiers for two or more different address spaces."
-  if (Type.getAddressSpace() != LangAS::Default) {
-    S.Diag(Attr.getLoc(), diag::err_attribute_address_multiple_qualifiers);
-    Attr.setInvalid();
-    return;
-  }
-
   // ISO/IEC TR 18037 S5.3 (amending C99 6.7.3): "A function type shall not be
   // qualified by an address-space qualifier."
   if (Type->isFunctionType()) {
     S.Diag(Attr.getLoc(), diag::err_attribute_address_function_type);
     Attr.setInvalid();
     return;
   }
 
if (Attr.getKind() == AttributeList::AT_AddressSpace) {
     case AttributeList::AT_OpenCLGenericAddressSpace:
       ASIdx = LangAS::opencl_generic; break;
     case AttributeList::AT_OpenCLPrivateAddressSpace:
       ASIdx = LangAS::opencl_private; break;
     default:
       llvm_unreachable("Invalid address space");
     }
 
+    // If this type is already address space qualified with a different
+    // address space, reject it.
+    // ISO/IEC TR 18037 S5.3 (amending C99 6.7.3): "No type shall be qualified by
+    // qualifiers for two or more different address spaces."
+    if (Type.getAddressSpace() != LangAS::Default) {
+      if (Type.getAddressSpace() != ASIdx) {
+        S.Diag(Attr.getLoc(), diag::err_attribute_address_multiple_qualifiers);
+        Attr.setInvalid();
+        return;
+      } else
+        // Emit a warning if they are identical; it's likely unintended.
+        S.Diag(Attr.getLoc(),
+               diag::warn_attribute_address_multiple_identical_qualifiers);
+    }
+
     Type = S.Context.getAddrSpaceQualType(Type, ASIdx);
   }
 }
 
 /// Does this type have a "direct" ownership qualifier?  That is,
 /// is it written like "__strong id", as opposed to something like
 /// "typeof(foo)", where that happens to be strong?
 static bool hasDirectOwnershipQualifier(QualType type) {

test/Sema/address_spaces.c

 // RUN: %clang_cc1 %s -fsyntax-only -verify
 
 #define _AS1 __attribute__((address_space(1)))
 #define _AS2 __attribute__((address_space(2)))
 #define _AS3 __attribute__((address_space(3)))
 
 void bar(_AS2 int a); // expected-error {{parameter may not be qualified with an address space}}
 
 void foo(_AS3 float *a,
          _AS1 float b) // expected-error {{parameter may not be qualified with an address space}}
 {
   _AS2 *x;// expected-warning {{type specifier missing, defaults to 'int'}}
   _AS1 float * _AS2 *B;
 
   int _AS1 _AS2 *Y;   // expected-error {{multiple address spaces specified for type}}
   int *_AS1 _AS2 *Z;  // expected-error {{multiple address spaces specified for type}}
+  int *_AS1 _AS1 *M;  // expected-warning {{multiple identical address spaces specified for type}}
 
   _AS1 int local;     // expected-error {{automatic variable qualified with an address space}}
   _AS1 int array[5];  // expected-error {{automatic variable qualified with an address space}}
   _AS1 int arrarr[5][5]; // expected-error {{automatic variable qualified with an address space}}
 
   __attribute__((address_space(-1))) int *_boundsA; // expected-error {{address space is negative}}
   __attribute__((address_space(0x7FFFFF))) int *_boundsB; // expected-error {{address space is larger than the maximum supported}}
   __attribute__((address_space(0x1000000))) int *_boundsC; // expected-error {{address space is larger than the maximum supported}}

test/SemaOpenCL/address-spaces.cl

 #endif
 
 void func_multiple_addr(void) {
   typedef __private int private_int_t;
   __private __local int var1;   // expected-error {{multiple address spaces specified for type}}
   __private __local int *var2;  // expected-error {{multiple address spaces specified for type}}
   __local private_int_t var3;   // expected-error {{multiple address spaces specified for type}}
   __local private_int_t *var4;  // expected-error {{multiple address spaces specified for type}}
+  __private private_int_t var5; // expected-warning {{multiple identical address spaces specified for type}}
+  __private private_int_t *var6;// expected-warning {{multiple identical address spaces specified for type}}
 }