Skip to content

Commit 7bd9540

Browse files
seven-milesmeenai
authored andcommitted
[CIR][CodeGen][Lowering] Better handling of alloca address space with unified AS (llvm#682)
`TargetCodeGenInfo::getASTAllocaAddressSpace` is a bad design because it requires the targets return `LangAS`, which enforce the targets to consider which languages could be their upstream and unnecessarily complicate the target info. Unified AS in CIR is a better abstraction level for this kind of target info. Apart from that, the languages also has some requirements on the result address space of alloca. ```cpp void func() { int x; // Here, the AS of pointer `&x` is the alloca AS defined by the language. } ``` When we have inconsistency between the alloca AS defined by the language and the one from target info, we have to perform `addrspacecast` from the target one to the language one. This PR includes * New method `CGM.getLangTempAllocaAddressSpace` which explicitly specifies the alloca address space defined by languages. It replaces the vague `LangAS::Default` in the AS comparisons from OG CodeGen. * Replace `getASTAllocaAddressSpace` with `getCIRAllocaAddressSpace`, which returns CIR unified AS. * Also use `getCIRAllocaAddressSpace` as the result address space of `buildAlloca`. * Fix the lowering of `cir.alloca` operation to be address-space-aware. We don't perform any `addrspacecast` in this PR, i.e. all the related code paths still remain NYI and it's fine. That's because we don't even have a `(language, target)` pair holding the inconsistency. After these changes, in the previous OpenCL testcases we will see all the alloca pointers turning into private AS, without any `addrspacecast`.
1 parent 5d08129 commit 7bd9540

File tree

9 files changed

+54
-20
lines changed

9 files changed

+54
-20
lines changed

clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -1100,7 +1100,7 @@ RValue CIRGenFunction::buildBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
11001100
// the AST level this is handled within CreateTempAlloca et al., but for the
11011101
// builtin / dynamic alloca we have to handle it here.
11021102
assert(!MissingFeatures::addressSpace());
1103-
auto AAS = builder.getAddrSpaceAttr(getASTAllocaAddressSpace());
1103+
auto AAS = getCIRAllocaAddressSpace();
11041104
auto EAS = builder.getAddrSpaceAttr(
11051105
E->getType()->getPointeeType().getAddressSpace());
11061106
if (EAS != AAS) {

clang/lib/CIR/CodeGen/CIRGenExpr.cpp

+7-2
Original file line numberDiff line numberDiff line change
@@ -2752,7 +2752,10 @@ mlir::Value CIRGenFunction::buildAlloca(StringRef name, mlir::Type ty,
27522752
mlir::Location loc, CharUnits alignment,
27532753
mlir::OpBuilder::InsertPoint ip,
27542754
mlir::Value arraySize) {
2755-
auto localVarPtrTy = mlir::cir::PointerType::get(builder.getContext(), ty);
2755+
// CIR uses its own alloca AS rather than follow the target data layout like
2756+
// original CodeGen. The data layout awareness should be done in the lowering
2757+
// pass instead.
2758+
auto localVarPtrTy = builder.getPointerTo(ty, getCIRAllocaAddressSpace());
27562759
auto alignIntAttr = CGM.getSize(alignment);
27572760

27582761
mlir::Value addr;
@@ -2977,7 +2980,9 @@ Address CIRGenFunction::CreateTempAlloca(mlir::Type Ty, CharUnits Align,
29772980
// be different from the type defined by the language. For example,
29782981
// in C++ the auto variables are in the default address space. Therefore
29792982
// cast alloca to the default address space when necessary.
2980-
if (getASTAllocaAddressSpace() != LangAS::Default) {
2983+
if (auto ASTAS =
2984+
builder.getAddrSpaceAttr(CGM.getLangTempAllocaAddressSpace());
2985+
getCIRAllocaAddressSpace() != ASTAS) {
29812986
llvm_unreachable("Requires address space cast which is NYI");
29822987
}
29832988
return Address(V, Ty, Align);

clang/lib/CIR/CodeGen/CIRGenModule.cpp

+11-1
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ CIRGenModule::CIRGenModule(mlir::MLIRContext &context,
159159
AllocaInt8PtrTy = UInt8PtrTy;
160160
// TODO: GlobalsInt8PtrTy
161161
// TODO: ConstGlobalsPtrTy
162-
ASTAllocaAddressSpace = getTargetCIRGenInfo().getASTAllocaAddressSpace();
162+
CIRAllocaAddressSpace = getTargetCIRGenInfo().getCIRAllocaAddressSpace();
163163

164164
PtrDiffTy = ::mlir::cir::IntType::get(
165165
builder.getContext(), astCtx.getTargetInfo().getMaxPointerWidth(),
@@ -1404,6 +1404,16 @@ LangAS CIRGenModule::getGlobalConstantAddressSpace() const {
14041404
return LangAS::Default;
14051405
}
14061406

1407+
// TODO(cir): this could be a common AST helper for both CIR and LLVM codegen.
1408+
LangAS CIRGenModule::getLangTempAllocaAddressSpace() const {
1409+
if (getLangOpts().OpenCL)
1410+
return LangAS::opencl_private;
1411+
if (getLangOpts().SYCLIsDevice || getLangOpts().CUDAIsDevice ||
1412+
(getLangOpts().OpenMP && getLangOpts().OpenMPIsTargetDevice))
1413+
llvm_unreachable("NYI");
1414+
return LangAS::Default;
1415+
}
1416+
14071417
static mlir::cir::GlobalOp
14081418
generateStringLiteral(mlir::Location loc, mlir::TypedAttr C,
14091419
mlir::cir::GlobalLinkageKind LT, CIRGenModule &CGM,

clang/lib/CIR/CodeGen/CIRGenModule.h

+6
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,12 @@ class CIRGenModule : public CIRGenTypeCache {
335335
/// in AST is always in default address space.
336336
LangAS getGlobalConstantAddressSpace() const;
337337

338+
/// Returns the address space for temporary allocations in the language. This
339+
/// ensures that the allocated variable's address space matches the
340+
/// expectations of the AST, rather than using the target's allocation address
341+
/// space, which may lead to type mismatches in other parts of the IR.
342+
LangAS getLangTempAllocaAddressSpace() const;
343+
338344
/// Set attributes which are common to any form of a global definition (alias,
339345
/// Objective-C method, function, global variable).
340346
///

clang/lib/CIR/CodeGen/CIRGenTypeCache.h

+4-8
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "clang/AST/CharUnits.h"
1919
#include "clang/Basic/AddressSpaces.h"
2020
#include "clang/CIR/Dialect/IR/CIRTypes.h"
21+
#include "clang/CIR/Dialect/IR/CIRAttrs.h"
2122
#include "clang/CIR/MissingFeatures.h"
2223

2324
namespace cir {
@@ -108,7 +109,7 @@ struct CIRGenTypeCache {
108109
// unsigned char SizeAlignInBytes;
109110
// };
110111

111-
clang::LangAS ASTAllocaAddressSpace;
112+
mlir::cir::AddressSpaceAttr CIRAllocaAddressSpace;
112113

113114
// clang::CharUnits getSizeSize() const {
114115
// return clang::CharUnits::fromQuantity(SizeSizeInBytes);
@@ -123,13 +124,8 @@ struct CIRGenTypeCache {
123124
return clang::CharUnits::fromQuantity(PointerAlignInBytes);
124125
}
125126

126-
clang::LangAS getASTAllocaAddressSpace() const {
127-
// Address spaces are not yet fully supported, but the usage of the default
128-
// alloca address space can be used for now only for comparison with the
129-
// default address space.
130-
assert(!MissingFeatures::addressSpace());
131-
assert(ASTAllocaAddressSpace == clang::LangAS::Default);
132-
return ASTAllocaAddressSpace;
127+
mlir::cir::AddressSpaceAttr getCIRAllocaAddressSpace() const {
128+
return CIRAllocaAddressSpace;
133129
}
134130
};
135131

clang/lib/CIR/CodeGen/TargetInfo.cpp

+6
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,12 @@ class CommonSPIRTargetCIRGenInfo : public TargetCIRGenInfo {
263263
CommonSPIRTargetCIRGenInfo(std::unique_ptr<ABIInfo> ABIInfo)
264264
: TargetCIRGenInfo(std::move(ABIInfo)) {}
265265

266+
mlir::cir::AddressSpaceAttr getCIRAllocaAddressSpace() const override {
267+
return mlir::cir::AddressSpaceAttr::get(
268+
&getABIInfo().CGT.getMLIRContext(),
269+
mlir::cir::AddressSpaceAttr::Kind::offload_private);
270+
}
271+
266272
unsigned getOpenCLKernelCallingConv() const override {
267273
return llvm::CallingConv::SPIR_KERNEL;
268274
}

clang/lib/CIR/CodeGen/TargetInfo.h

+5-3
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,11 @@ class TargetCIRGenInfo {
6262
std::vector<LValue> &ResultRegDests,
6363
std::string &AsmString, unsigned NumOutputs) const {}
6464

65-
/// Get the AST address space for alloca.
66-
virtual clang::LangAS getASTAllocaAddressSpace() const {
67-
return clang::LangAS::Default;
65+
/// Get the CIR address space for alloca.
66+
virtual mlir::cir::AddressSpaceAttr getCIRAllocaAddressSpace() const {
67+
// Return the null attribute, which means the target does not care about the
68+
// alloca address space.
69+
return {};
6870
}
6971

7072
/// Perform address space cast of an expression of pointer type.

clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -999,7 +999,8 @@ class CIRAllocaLowering
999999
typeConverter->convertType(rewriter.getIndexType()),
10001000
rewriter.getIntegerAttr(rewriter.getIndexType(), 1));
10011001
auto elementTy = getTypeConverter()->convertType(op.getAllocaType());
1002-
auto resultTy = mlir::LLVM::LLVMPointerType::get(getContext());
1002+
auto resultTy = getTypeConverter()->convertType(op.getResult().getType());
1003+
// TODO: Verification between the CIR alloca AS and the one from data layout
10031004
rewriter.replaceOpWithNewOp<mlir::LLVM::AllocaOp>(
10041005
op, resultTy, elementTy, size, op.getAlignmentAttr().getInt());
10051006
return mlir::success();

clang/test/CIR/CodeGen/OpenCL/addrspace-alloca.cl

+12-4
Original file line numberDiff line numberDiff line change
@@ -7,20 +7,28 @@
77
// CIR: cir.func @func(%arg0: !cir.ptr<!s32i, addrspace(offload_local)>
88
// LLVM: @func(ptr addrspace(3)
99
kernel void func(local int *p) {
10-
// CIR-NEXT: %[[#ALLOCA_P:]] = cir.alloca !cir.ptr<!s32i, addrspace(offload_local)>, !cir.ptr<!cir.ptr<!s32i, addrspace(offload_local)>>, ["p", init] {alignment = 8 : i64}
10+
// CIR-NEXT: %[[#ALLOCA_P:]] = cir.alloca !cir.ptr<!s32i, addrspace(offload_local)>, !cir.ptr<!cir.ptr<!s32i, addrspace(offload_local)>, addrspace(offload_private)>, ["p", init] {alignment = 8 : i64}
1111
// LLVM-NEXT: %[[#ALLOCA_P:]] = alloca ptr addrspace(3), i64 1, align 8
1212

1313
int x;
14-
// CIR-NEXT: %[[#ALLOCA_X:]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["x"] {alignment = 4 : i64}
14+
// CIR-NEXT: %[[#ALLOCA_X:]] = cir.alloca !s32i, !cir.ptr<!s32i, addrspace(offload_private)>, ["x"] {alignment = 4 : i64}
1515
// LLVM-NEXT: %[[#ALLOCA_X:]] = alloca i32, i64 1, align 4
1616

1717
global char *b;
18-
// CIR-NEXT: %[[#ALLOCA_B:]] = cir.alloca !cir.ptr<!s8i, addrspace(offload_global)>, !cir.ptr<!cir.ptr<!s8i, addrspace(offload_global)>>, ["b"] {alignment = 8 : i64}
18+
// CIR-NEXT: %[[#ALLOCA_B:]] = cir.alloca !cir.ptr<!s8i, addrspace(offload_global)>, !cir.ptr<!cir.ptr<!s8i, addrspace(offload_global)>, addrspace(offload_private)>, ["b"] {alignment = 8 : i64}
1919
// LLVM-NEXT: %[[#ALLOCA_B:]] = alloca ptr addrspace(1), i64 1, align 8
2020

21+
private int *ptr;
22+
// CIR-NEXT: %[[#ALLOCA_PTR:]] = cir.alloca !cir.ptr<!s32i, addrspace(offload_private)>, !cir.ptr<!cir.ptr<!s32i, addrspace(offload_private)>, addrspace(offload_private)>, ["ptr"] {alignment = 8 : i64}
23+
// LLVM-NEXT: %[[#ALLOCA_PTR:]] = alloca ptr, i64 1, align 8
24+
2125
// Store of the argument `p`
22-
// CIR-NEXT: cir.store %arg0, %[[#ALLOCA_P]] : !cir.ptr<!s32i, addrspace(offload_local)>, !cir.ptr<!cir.ptr<!s32i, addrspace(offload_local)>>
26+
// CIR-NEXT: cir.store %arg0, %[[#ALLOCA_P]] : !cir.ptr<!s32i, addrspace(offload_local)>, !cir.ptr<!cir.ptr<!s32i, addrspace(offload_local)>, addrspace(offload_private)>
2327
// LLVM-NEXT: store ptr addrspace(3) %{{[0-9]+}}, ptr %[[#ALLOCA_P]], align 8
2428

29+
ptr = &x;
30+
// CIR-NEXT: cir.store %[[#ALLOCA_X]], %[[#ALLOCA_PTR]] : !cir.ptr<!s32i, addrspace(offload_private)>, !cir.ptr<!cir.ptr<!s32i, addrspace(offload_private)>, addrspace(offload_private)>
31+
// LLVM-NEXT: store ptr %[[#ALLOCA_X]], ptr %[[#ALLOCA_PTR]]
32+
2533
return;
2634
}

0 commit comments

Comments
 (0)