[SYCL] Prohibit arithmetic operations for incompatible pointers

[bader]

Summary

This change enables OpenCL diagnostics for the pointers annotated with
address space attribute SYCL mode.

[bader]

clang/lib/Sema/SemaExpr.cpp

10090

Alternative approach would be to remove S.getLangOpts().OpenCL to enable this check for all modes.

@Anastasia, do you know if it's safe to do?

[Anastasia]

clang/lib/Sema/SemaExpr.cpp

10090

If I look at embedded C (ISO/IEC TR 18037) s5.3 rules that we are following largely in Clang I believe this is a universal rule!

Especially looking at the followong statement:

Clause 6.5.6 - Additive operators, add another constraint paragraph:
For subtraction, if the two operands are pointers into different address spaces, the address spaces must overlap.

So I think that it should apply to at least OpenCL, C and C++. I am surprised though that this has not been fixed yet.

I am CCing @rjmccall and @jeroen.dobbelaere in case they have any more insights on this.

[rjmccall]

clang/lib/Sema/SemaExpr.cpp

10090

Yes, I agree, this should apply in all modes.

You should able to restructure the isAddressSpaceOverlapping function so that it can work directly on the address spaces of LHSPointeeTy and RHSPointeeTy; the code will be both cleaner and faster.

[bader]

Enable diagnostics for non-OpenCL modes and applied refactoring proposed by John.

[bader]

Thanks for the review.
I've enabled the diagnostics for all the modes.
I also applied the refactoring suggested by @rjmccall. Hopefully I understand it correctly.

[rjmccall]

Please add a C test case just using the address_space attribute.

clang/include/clang/AST/Type.h

1069 ↗

(On Diff #265487)

It's idiomatic to take QualType by value rather than const &.

Can you rewrite the PointerType method to delegate to this? Assuming it isn't completely dead, that is.

[bader]

• Added C test case with address_space attribute.
• Move isAddressSpaceOverlapping from PointerType to QualType.
• Move C++ test case to clang/test/SemaCXX

[bader]

clang/include/clang/AST/Type.h

1069 ↗

(On Diff #265487)

It isn't completely dead, but there were just a few uses of the PointerType method, so I've updated all of them to avoid code duplication in two classes.

[rjmccall]

LGTM

clang/include/clang/AST/Type.h

1069 ↗

(On Diff #265487)

Even better, thanks.

[bader]

Fix formatting in clang/test/Sema/address_spaces.c

clang/lib/Sema/SemaExpr.cpp

static bool checkArithmeticBinOpPointerOperands(Sema &S, SourceLocation Loc,
   bool isRHSPointer = RHSExpr->getType()->isAnyPointerType();
   if (!isLHSPointer && !isRHSPointer) return true;
 
   QualType LHSPointeeTy, RHSPointeeTy;
   if (isLHSPointer) LHSPointeeTy = LHSExpr->getType()->getPointeeType();
   if (isRHSPointer) RHSPointeeTy = RHSExpr->getType()->getPointeeType();
 
   // if both are pointers check if operation is valid wrt address spaces
-  if (S.getLangOpts().OpenCL && isLHSPointer && isRHSPointer) {
+  if ((S.getLangOpts().OpenCL || S.getLangOpts().SYCLIsDevice) &&
+      isLHSPointer && isRHSPointer) {
     const PointerType *lhsPtr = LHSExpr->getType()->castAs<PointerType>();
     const PointerType *rhsPtr = RHSExpr->getType()->castAs<PointerType>();
     if (!lhsPtr->isAddressSpaceOverlapping(*rhsPtr)) {
       S.Diag(Loc,
              diag::err_typecheck_op_on_nonoverlapping_address_space_pointers)
           << LHSExpr->getType() << RHSExpr->getType() << 1 /*arithmetic op*/
           << LHSExpr->getSourceRange() << RHSExpr->getSourceRange();
       return false;

clang/test/SemaSYCL/address-space-arithmetic.cpp

+// RUN: %clang_cc1 -fsycl -fsycl-is-device -fsyntax-only -verify %s
+
+int *foo(__attribute__((opencl_private)) int *p,
+         __attribute__((opencl_local)) int *l) {
+  return p - l; // expected-error {{arithmetic operation with operands of type  ('__private int *' and '__local int *') which are pointers to }}
+}