[SYCL] Refactor address space handling in CodeGen library (#2864)

This patch reverts DPC++ customizations in CodeGen library enabling generic
address space as default for SYCL mode. Based on feedback from the
community there is a better way to get similar results. CodeGen
TargetInfo object is be used to set default address space for global
variables and stack allocations.

Author: bader

clang/lib/Basic/Targets/SPIR.h

@@ -114,6 +114,16 @@ class LLVM_LIBRARY_VISIBILITY SPIRTargetInfo : public TargetInfo {
     return CC_SpirFunction;
   }
 
+  llvm::Optional<LangAS> getConstantAddressSpace() const override {
+    // If we assign "opencl_constant" address space the following code becomes
+    // illegal, because it can't be cast to any other address space:
+    //
+    //   const char *getLiteral() {
+    //     return "AB";
+    //   }
+    return LangAS::opencl_global;
+  }
+
   void setSupportedOpenCLOpts() override {
     // Assume all OpenCL extensions and optional core features are supported
     // for SPIR since it is a generic target.

clang/lib/CodeGen/CGCall.cpp

@@ -4771,17 +4771,6 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,
             V->getType()->isIntegerTy())
           V = Builder.CreateZExt(V, ArgInfo.getCoerceToType());
 
-        if (FirstIRArg < IRFuncTy->getNumParams()) {
-          const auto *LHSPtrTy =
-              dyn_cast_or_null<llvm::PointerType>(V->getType());
-          const auto *RHSPtrTy = dyn_cast_or_null<llvm::PointerType>(
-              IRFuncTy->getParamType(FirstIRArg));
-          if (LHSPtrTy && RHSPtrTy &&
-              LHSPtrTy->getAddressSpace() != RHSPtrTy->getAddressSpace())
-            V = Builder.CreateAddrSpaceCast(V,
-                                            IRFuncTy->getParamType(FirstIRArg));
-        }
-
         // If the argument doesn't match, perform a bitcast to coerce it.  This
         // can happen due to trivial type mismatches.
         if (FirstIRArg < IRFuncTy->getNumParams() &&
@@ -5002,20 +4991,6 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,
   if (!CallArgs.getCleanupsToDeactivate().empty())
     deactivateArgCleanupsBeforeCall(*this, CallArgs);
 
-  // Addrspace cast to generic if necessary
-  for (unsigned i = 0; i < IRFuncTy->getNumParams(); ++i) {
-    if (auto *PtrTy = dyn_cast<llvm::PointerType>(IRCallArgs[i]->getType())) {
-      auto *ExpectedPtrType =
-          cast<llvm::PointerType>(IRFuncTy->getParamType(i));
-      unsigned ValueAS = PtrTy->getAddressSpace();
-      unsigned ExpectedAS = ExpectedPtrType->getAddressSpace();
-      if (ValueAS != ExpectedAS) {
-        IRCallArgs[i] = Builder.CreatePointerBitCastOrAddrSpaceCast(
-            IRCallArgs[i], ExpectedPtrType);
-      }
-    }
-  }
-
   // Assert that the arguments we computed match up.  The IR verifier
   // will catch this, but this is a common enough source of problems
   // during IRGen changes that it's way better for debugging to catch

clang/lib/CodeGen/CGClass.cpp

@@ -342,7 +342,7 @@ Address CodeGenFunction::GetAddressOfBaseClass(
       EmitTypeCheck(TCK_Upcast, Loc, Value.getPointer(),
                     DerivedTy, DerivedAlign, SkippedChecks);
     }
-    return Builder.CreatePointerBitCastOrAddrSpaceCast(Value, BasePtrTy);
+    return Builder.CreateBitCast(Value, BasePtrTy);
   }
 
   llvm::BasicBlock *origBB = nullptr;

clang/lib/CodeGen/CGDecl.cpp

@@ -1642,8 +1642,12 @@ CodeGenFunction::EmitAutoVarAlloca(const VarDecl &D) {
     CGM.generateIntelFPGAAnnotation(&D, AnnotStr);
     if (!AnnotStr.empty()) {
       llvm::Value *V = address.getPointer();
-      EmitAnnotationCall(CGM.getIntrinsic(llvm::Intrinsic::var_annotation),
-                         Builder.CreateBitCast(V, CGM.Int8PtrTy, V->getName()),
+      llvm::Type *DestPtrTy = llvm::PointerType::getInt8PtrTy(
+          CGM.getLLVMContext(), address.getAddressSpace());
+      llvm::Value *Arg = Builder.CreateBitCast(V, DestPtrTy, V->getName());
+      if (address.getAddressSpace() != 0)
+        Arg = Builder.CreateAddrSpaceCast(Arg, CGM.Int8PtrTy, V->getName());
+      EmitAnnotationCall(CGM.getIntrinsic(llvm::Intrinsic::var_annotation), Arg,
                          AnnotStr, D.getLocation());
     }
   }

clang/lib/CodeGen/CGExpr.cpp

@@ -1125,8 +1125,10 @@ Address CodeGenFunction::EmitPointerWithAlignment(const Expr *E,
                                       CodeGenFunction::CFITCK_UnrelatedCast,
                                       CE->getBeginLoc());
         }
-        return Builder.CreatePointerBitCastOrAddrSpaceCast(
-            Addr, ConvertType(E->getType()));
+        return CE->getCastKind() != CK_AddressSpaceConversion
+                   ? Builder.CreateBitCast(Addr, ConvertType(E->getType()))
+                   : Builder.CreateAddrSpaceCast(Addr,
+                                                 ConvertType(E->getType()));
       }
       break;
 
@@ -1855,16 +1857,6 @@ void CodeGenFunction::EmitStoreOfScalar(llvm::Value *Value, Address Addr,
     return;
   }
 
-  if (auto *PtrTy = dyn_cast<llvm::PointerType>(Value->getType())) {
-    auto *ExpectedPtrType =
-        cast<llvm::PointerType>(Addr.getType()->getElementType());
-    unsigned ValueAS = PtrTy->getAddressSpace();
-    unsigned ExpectedAS = ExpectedPtrType->getAddressSpace();
-    if (ValueAS != ExpectedAS) {
-      Value =
-          Builder.CreatePointerBitCastOrAddrSpaceCast(Value, ExpectedPtrType);
-    }
-  }
   llvm::StoreInst *Store = Builder.CreateStore(Value, Addr, Volatile);
   if (isNontemporal) {
     llvm::MDNode *Node =
@@ -4614,39 +4606,14 @@ EmitConditionalOperatorLValue(const AbstractConditionalOperator *expr) {
   EmitBlock(contBlock);
 
   if (lhs && rhs) {
-    llvm::Value *lhsPtr = lhs->getPointer(*this);
-    llvm::Value *rhsPtr = rhs->getPointer(*this);
-    if (rhsPtr->getType() != lhsPtr->getType()) {
-      if (!getLangOpts().SYCLIsDevice)
-        llvm_unreachable(
-            "Unable to find a common address space for two pointers.");
-
-      auto CastToAS = [](llvm::Value *V, llvm::BasicBlock *BB, unsigned AS) {
-        auto *Ty = cast<llvm::PointerType>(V->getType());
-        if (Ty->getAddressSpace() == AS)
-          return V;
-        llvm::IRBuilder<> Builder(BB->getTerminator());
-        auto *TyAS = llvm::PointerType::get(Ty->getElementType(), AS);
-        return Builder.CreatePointerBitCastOrAddrSpaceCast(V, TyAS);
-      };
-
-      // Language rules define if it is legal to cast from one address space
-      // to another, and which address space we should use as a "common
-      // denominator". In SYCL, generic address space overlaps with all other
-      // address spaces.
-      unsigned GenericAS =
-          getContext().getTargetAddressSpace(LangAS::opencl_generic);
-
-      lhsPtr = CastToAS(lhsPtr, lhsBlock, GenericAS);
-      rhsPtr = CastToAS(rhsPtr, rhsBlock, GenericAS);
-    }
-    llvm::PHINode *phi = Builder.CreatePHI(lhsPtr->getType(), 2, "cond-lvalue");
-    phi->addIncoming(lhsPtr, lhsBlock);
-    phi->addIncoming(rhsPtr, rhsBlock);
+    llvm::PHINode *phi =
+        Builder.CreatePHI(lhs->getPointer(*this)->getType(), 2, "cond-lvalue");
+    phi->addIncoming(lhs->getPointer(*this), lhsBlock);
+    phi->addIncoming(rhs->getPointer(*this), rhsBlock);
     Address result(phi, std::min(lhs->getAlignment(), rhs->getAlignment()));
     AlignmentSource alignSource =
-        std::max(lhs->getBaseInfo().getAlignmentSource(),
-                 rhs->getBaseInfo().getAlignmentSource());
+      std::max(lhs->getBaseInfo().getAlignmentSource(),
+               rhs->getBaseInfo().getAlignmentSource());
     TBAAAccessInfo TBAAInfo = CGM.mergeTBAAInfoForConditionalOperator(
         lhs->getTBAAInfo(), rhs->getTBAAInfo());
     return MakeAddrLValue(result, expr->getType(), LValueBaseInfo(alignSource),

clang/lib/CodeGen/CGExprAgg.cpp

@@ -498,13 +498,7 @@ void AggExprEmitter::EmitArrayInit(Address DestPtr, llvm::ArrayType *AType,
       elementType.isTriviallyCopyableType(CGF.getContext())) {
     CodeGen::CodeGenModule &CGM = CGF.CGM;
     ConstantEmitter Emitter(CGF);
-    LangAS AS = ArrayQTy.getAddressSpace();
-    if (CGM.getLangOpts().SYCLIsDevice && AS == LangAS::Default) {
-      // SYCL's default AS is 'generic', which can't be used to define constant
-      // initializer data in. It is reasonable to keep it in the same AS
-      // as string literals.
-      AS = CGM.getStringLiteralAddressSpace();
-    }
+    LangAS AS = CGM.GetGlobalVarAddressSpace(/*VarDecl= */ nullptr);
     if (llvm::Constant *C = Emitter.tryEmitForInitializer(E, AS, ArrayQTy)) {
       auto GV = new llvm::GlobalVariable(
           CGM.getModule(), C->getType(),

clang/lib/CodeGen/CGExprScalar.cpp

@@ -1964,26 +1964,10 @@ Value *ScalarExprEmitter::VisitCastExpr(CastExpr *CE) {
     Value *Src = Visit(const_cast<Expr*>(E));
     llvm::Type *SrcTy = Src->getType();
     llvm::Type *DstTy = ConvertType(DestTy);
-    bool NeedAddrspaceCast = false;
     if (SrcTy->isPtrOrPtrVectorTy() && DstTy->isPtrOrPtrVectorTy() &&
         SrcTy->getPointerAddressSpace() != DstTy->getPointerAddressSpace()) {
-      // If we have the same address space in AST, which is then codegen'ed to
-      // different address spaces in IR, then an address space cast should be
-      // valid.
-      //
-      // This is the case for SYCL, where both types have Default address space
-      // in AST, but in IR one of them may be in opencl_private, and another in
-      // opencl_generic address space:
-      //
-      //   int arr[5];    // automatic variable, default AS in AST,
-      //                  // private AS in IR
-      //
-      //   char* p = arr; // default AS in AST, generic AS in IR
-      //
-      if (E->getType().getAddressSpace() != DestTy.getAddressSpace())
-        llvm_unreachable("wrong cast for pointers in different address spaces"
-                         "(must be an address space cast)!");
-      NeedAddrspaceCast = true;
+      llvm_unreachable("wrong cast for pointers in different address spaces"
+                       "(must be an address space cast)!");
     }
 
     if (CGF.SanOpts.has(SanitizerKind::CFIUnrelatedCast)) {
@@ -2022,14 +2006,6 @@ Value *ScalarExprEmitter::VisitCastExpr(CastExpr *CE) {
       }
     }
 
-    if (NeedAddrspaceCast) {
-      llvm::Type *SrcPointeeTy = Src->getType()->getPointerElementType();
-      llvm::Type *SrcNewAS = llvm::PointerType::get(
-          SrcPointeeTy, cast<llvm::PointerType>(DstTy)->getAddressSpace());
-
-      Src = Builder.CreateAddrSpaceCast(Src, SrcNewAS);
-    }
-
     // If Src is a fixed vector and Dst is a scalable vector, and both have the
     // same element type, use the llvm.experimental.vector.insert intrinsic to
     // perform the bitcast.
@@ -2966,53 +2942,6 @@ Value *ScalarExprEmitter::VisitUnaryImag(const UnaryOperator *E) {
 //                           Binary Operators
 //===----------------------------------------------------------------------===//
 
-static Value *insertAddressSpaceCast(Value *V, unsigned NewAS) {
-  auto *VTy = cast<llvm::PointerType>(V->getType());
-  if (VTy->getAddressSpace() == NewAS)
-    return V;
-
-  llvm::PointerType *VTyNewAS =
-      llvm::PointerType::get(VTy->getElementType(), NewAS);
-
-  if (auto *Constant = dyn_cast<llvm::Constant>(V))
-    return llvm::ConstantExpr::getAddrSpaceCast(Constant, VTyNewAS);
-
-  llvm::Instruction *NewV =
-      new llvm::AddrSpaceCastInst(V, VTyNewAS, V->getName() + ".ascast");
-  NewV->insertAfter(cast<llvm::Instruction>(V));
-  return NewV;
-}
-
-static void ensureSameAddrSpace(Value *&RHS, Value *&LHS,
-                                bool CanInsertAddrspaceCast,
-                                const LangOptions &Opts,
-                                const ASTContext &Context) {
-  if (RHS->getType() == LHS->getType())
-    return;
-
-  auto *RHSTy = dyn_cast<llvm::PointerType>(RHS->getType());
-  auto *LHSTy = dyn_cast<llvm::PointerType>(LHS->getType());
-  if (!RHSTy || !LHSTy || RHSTy->getAddressSpace() == LHSTy->getAddressSpace())
-    return;
-
-  if (!CanInsertAddrspaceCast)
-    // Pointers have different address spaces and we cannot do anything with
-    // this.
-    llvm_unreachable("Pointers are expected to have the same address space.");
-
-  // Language rules define if it is legal to cast from one address space to
-  // another, and which address space we should use as a "common
-  // denominator". In SYCL, generic address space overlaps with all other
-  // address spaces.
-  if (Opts.SYCLIsDevice) {
-    unsigned GenericAS = Context.getTargetAddressSpace(LangAS::opencl_generic);
-    RHS = insertAddressSpaceCast(RHS, GenericAS);
-    LHS = insertAddressSpaceCast(LHS, GenericAS);
-  } else
-    llvm_unreachable("Unable to find a common address space for "
-                     "two pointers.");
-}
-
 BinOpInfo ScalarExprEmitter::EmitBinOps(const BinaryOperator *E) {
   TestAndClearIgnoreResultAssign();
   BinOpInfo Result;
@@ -4134,14 +4063,6 @@ Value *ScalarExprEmitter::EmitCompare(const BinaryOperator *E,
           RHS = Builder.CreateStripInvariantGroup(RHS);
       }
 
-      // Expression operands may have the same addrspace in AST, but different
-      // addrspaces in LLVM IR, in which case an addrspacecast should be valid.
-      bool CanInsertAddrspaceCast =
-             LHSTy.getAddressSpace() == RHSTy.getAddressSpace();
-
-      ensureSameAddrSpace(RHS, LHS, CanInsertAddrspaceCast, CGF.getLangOpts(),
-                          CGF.getContext());
-
       Result = Builder.CreateICmp(UICmpOpc, LHS, RHS, "cmp");
     }
 
@@ -4513,6 +4434,7 @@ static bool isCheapEnoughToEvaluateUnconditionally(const Expr *E,
   // exist in the source-level program.
 }
 
+
 Value *ScalarExprEmitter::
 VisitAbstractConditionalOperator(const AbstractConditionalOperator *E) {
   TestAndClearIgnoreResultAssign();
@@ -4621,15 +4543,6 @@ VisitAbstractConditionalOperator(const AbstractConditionalOperator *E) {
       assert(!RHS && "LHS and RHS types must match");
       return nullptr;
     }
-
-    // Expressions may have the same addrspace in AST, but different address
-    // space in LLVM IR, in which case an addrspacecast should be valid.
-    bool CanInsertAddrspaceCast = rhsExpr->getType().getAddressSpace() ==
-                                  lhsExpr->getType().getAddressSpace();
-
-    ensureSameAddrSpace(RHS, LHS, CanInsertAddrspaceCast, CGF.getLangOpts(),
-                        CGF.getContext());
-
     return Builder.CreateSelect(CondV, LHS, RHS, "cond");
   }
 
@@ -4664,14 +4577,6 @@ VisitAbstractConditionalOperator(const AbstractConditionalOperator *E) {
   if (!RHS)
     return LHS;
 
-  // Expressions may have the same addrspace in AST, but different address
-  // space in LLVM IR, in which case an addrspacecast should be valid.
-  bool CanInsertAddrspaceCast = rhsExpr->getType().getAddressSpace() ==
-                                lhsExpr->getType().getAddressSpace();
-
-  ensureSameAddrSpace(RHS, LHS, CanInsertAddrspaceCast, CGF.getLangOpts(),
-                      CGF.getContext());
-
   // Create a PHI node for the real part.
   llvm::PHINode *PN = Builder.CreatePHI(LHS->getType(), 2, "cond");
   PN->addIncoming(LHS, LHSBlock);

clang/lib/CodeGen/CGStmt.cpp

@@ -1211,35 +1211,12 @@ void CodeGenFunction::EmitReturnStmt(const ReturnStmt &S) {
     // If this function returns a reference, take the address of the expression
     // rather than the value.
     RValue Result = EmitReferenceBindingToExpr(RV);
-    llvm::Value *Val = Result.getScalarVal();
-    if (auto *PtrTy = dyn_cast<llvm::PointerType>(Val->getType())) {
-      auto *ExpectedPtrType =
-          cast<llvm::PointerType>(ReturnValue.getType()->getElementType());
-      unsigned ValueAS = PtrTy->getAddressSpace();
-      unsigned ExpectedAS = ExpectedPtrType->getAddressSpace();
-      if (ValueAS != ExpectedAS) {
-        Val = Builder.CreatePointerBitCastOrAddrSpaceCast(Val, ExpectedPtrType);
-      }
-    }
-    Builder.CreateStore(Val, ReturnValue);
+    Builder.CreateStore(Result.getScalarVal(), ReturnValue);
   } else {
     switch (getEvaluationKind(RV->getType())) {
     case TEK_Scalar:
-    {
-      llvm::Value *Val = EmitScalarExpr(RV);
-      if (auto *PtrTy = dyn_cast<llvm::PointerType>(Val->getType())) {
-        auto *ExpectedPtrType =
-            cast<llvm::PointerType>(ReturnValue.getType()->getElementType());
-        unsigned ValueAS = PtrTy->getAddressSpace();
-        unsigned ExpectedAS = ExpectedPtrType->getAddressSpace();
-        if (ValueAS != ExpectedAS) {
-          Val =
-              Builder.CreatePointerBitCastOrAddrSpaceCast(Val, ExpectedPtrType);
-        }
-      }
-      Builder.CreateStore(Val, ReturnValue);
+      Builder.CreateStore(EmitScalarExpr(RV), ReturnValue);
       break;
-    }
     case TEK_Complex:
       EmitComplexExprIntoLValue(RV, MakeAddrLValue(ReturnValue, RV->getType()),
                                 /*isInit*/ true);