Skip to content

Commit df23476

Browse files
RiverDaveRiverDave
committed
address comments
1 parent d28bd08 commit df23476

File tree

7 files changed

+113
-16
lines changed

7 files changed

+113
-16
lines changed

‎clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -60,18 +60,18 @@ static RValue emitBuiltinBitOp(CIRGenFunction &cgf, const CallExpr *e,
6060

6161
// Initialize the alloca with the given size and alignment according to the lang
6262
// opts. Supporting only the trivial non-initialization for now.
63-
static void initializeAlloca(CIRGenFunction &CGF,
64-
[[maybe_unused]] mlir::Value AllocaAddr,
65-
[[maybe_unused]] mlir::Value Size,
66-
[[maybe_unused]] CharUnits AlignmentInBytes) {
63+
static void initializeAlloca(CIRGenFunction &cgf,
64+
[[maybe_unused]] mlir::Value allocaAddr,
65+
[[maybe_unused]] mlir::Value size,
66+
[[maybe_unused]] CharUnits alignmentInBytes) {
6767

68-
switch (CGF.getLangOpts().getTrivialAutoVarInit()) {
68+
switch (cgf.getLangOpts().getTrivialAutoVarInit()) {
6969
case LangOptions::TrivialAutoVarInitKind::Uninitialized:
7070
// Nothing to initialize.
7171
return;
7272
case LangOptions::TrivialAutoVarInitKind::Zero:
7373
case LangOptions::TrivialAutoVarInitKind::Pattern:
74-
assert(false && "unexpected trivial auto var init kind NYI");
74+
cgf.cgm.errorNYI("trivial auto var init");
7575
return;
7676
}
7777
}
@@ -198,17 +198,16 @@ RValue CIRGenFunction::emitBuiltinExpr(const GlobalDecl &gd, unsigned builtinID,
198198
// default (e.g. in C / C++ auto vars are in the generic address space). At
199199
// the AST level this is handled within CreateTempAlloca et al., but for the
200200
// builtin / dynamic alloca we have to handle it here.
201-
assert(!cir::MissingFeatures::addressSpace());
202201
cir::AddressSpace aas = getCIRAllocaAddressSpace();
203202
cir::AddressSpace eas = cir::toCIRAddressSpace(
204203
e->getType()->getPointeeType().getAddressSpace());
205204
if (eas != aas) {
206-
assert(false && "Non-default address space for alloca NYI");
205+
cgm.errorNYI(e->getSourceRange(), "Non-default address space for alloca");
207206
}
208207

209208
// Bitcast the alloca to the expected type.
210209
return RValue::get(
211-
builder.createBitcast(allocaAddr, builder.getVoidPtrTy()));
210+
builder.createBitcast(allocaAddr, builder.getVoidPtrTy(aas)));
212211
}
213212

214213
case Builtin::BIcos:

‎clang/lib/CIR/CodeGen/CIRGenExpr.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2280,6 +2280,8 @@ Address CIRGenFunction::createTempAllocaWithoutCast(
22802280

22812281
/// This creates a alloca and inserts it into the entry block. The alloca is
22822282
/// casted to default address space if necessary.
2283+
// TODO(cir): Implement address space casting to match classic codegen's
2284+
// CreateTempAlloca behavior with DestLangAS parameter
22832285
Address CIRGenFunction::createTempAlloca(mlir::Type ty, CharUnits align,
22842286
mlir::Location loc, const Twine &name,
22852287
mlir::Value arraySize,

‎clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1867,7 +1867,7 @@ mlir::Value ScalarExprEmitter::VisitCastExpr(CastExpr *ce) {
18671867
Expr::EvalResult result;
18681868
if (subExpr->EvaluateAsRValue(result, cgf.getContext()) &&
18691869
result.Val.isNullPointer()) {
1870-
// If E has side effect, it is emitted even if its final result is a
1870+
// If e has side effect, it is emitted even if its final result is a
18711871
// null pointer. In that case, a DCE pass should be able to
18721872
// eliminate the useless instructions emitted during translating E.
18731873
if (result.HasSideEffects)

‎clang/lib/CIR/CodeGen/CIRGenModule.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1377,7 +1377,7 @@ LangAS CIRGenModule::getLangTempAllocaAddressSpace() const {
13771377

13781378
if (getLangOpts().SYCLIsDevice ||
13791379
(getLangOpts().OpenMP && getLangOpts().OpenMPIsTargetDevice))
1380-
llvm_unreachable("NYI");
1380+
errorNYI("SYCL or OpenMP temp address space");
13811381
return LangAS::Default;
13821382
}
13831383

‎clang/lib/CIR/CodeGen/TargetInfo.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ mlir::Value TargetCIRGenInfo::performAddrSpaceCast(
7777
// Since target may map different address spaces in AST to the same address
7878
// space, an address space conversion may end up as a bitcast.
7979
if (cir::GlobalOp globalOp = src.getDefiningOp<cir::GlobalOp>())
80-
llvm_unreachable("Global ops addrspace cast NYI");
80+
cgf.cgm.errorNYI("Global op addrspace cast");
8181
// Try to preserve the source's name to make IR more readable.
8282
return cgf.getBuilder().createAddrSpaceCast(src, destTy);
8383
}

‎clang/lib/CIR/Dialect/IR/CIRDialect.cpp‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,16 @@ LogicalResult cir::CastOp::verify() {
378378
mlir::Type resType = getType();
379379
mlir::Type srcType = getSrc().getType();
380380

381+
// Verify address space casts for pointer types. given that
382+
// casts for within a different address space are illegal.
383+
auto srcPtrTy = mlir::dyn_cast<cir::PointerType>(srcType);
384+
auto resPtrTy = mlir::dyn_cast<cir::PointerType>(resType);
385+
if (srcPtrTy && resPtrTy && (getKind() != cir::CastKind::address_space))
386+
if (srcPtrTy.getAddrSpace() != resPtrTy.getAddrSpace()) {
387+
return emitOpError() << "result type address space does not match the "
388+
"address space of the operand";
389+
}
390+
381391
if (mlir::isa<cir::VectorType>(srcType) &&
382392
mlir::isa<cir::VectorType>(resType)) {
383393
// Use the element type of the vector to verify the cast kind. (Except for
@@ -423,10 +433,7 @@ LogicalResult cir::CastOp::verify() {
423433
auto resPtrTy = mlir::dyn_cast<cir::PointerType>(resType);
424434

425435
if (srcPtrTy && resPtrTy) {
426-
if (srcPtrTy.getAddrSpace() != resPtrTy.getAddrSpace()) {
427-
return emitOpError() << "result type address space does not match the "
428-
"address space of the operand";
429-
}
436+
return success();
430437
}
431438

432439
return success();

‎clang/test/CIR/CodeGen/address-space-conversion.cpp‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
// RUN: %clang_cc1 -std=c++17 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o %t.cir
2+
// RUN: FileCheck --input-file=%t.cir %s -check-prefix=CIR
3+
// RUN: %clang_cc1 -std=c++17 -triple x86_64-unknown-linux-gnu -fclangir -emit-llvm %s -o %t.ll
4+
// RUN: FileCheck --input-file=%t.ll %s -check-prefix=LLVM
5+
// RUN: %clang_cc1 -std=c++17 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o %t.ll
6+
// RUN: FileCheck --input-file=%t.ll %s -check-prefix=OGCG
7+
8+
using pi1_t = int __attribute__((address_space(1))) *;
9+
using pi2_t = int __attribute__((address_space(2))) *;
10+
11+
using ri1_t = int __attribute__((address_space(1))) &;
12+
using ri2_t = int __attribute__((address_space(2))) &;
13+
14+
// CIR: cir.func dso_local @{{.*test_ptr.*}}
15+
// LLVM: define dso_local void @{{.*test_ptr.*}}
16+
// OGCG: define dso_local void @{{.*test_ptr.*}}
17+
void test_ptr() {
18+
pi1_t ptr1;
19+
pi2_t ptr2 = (pi2_t)ptr1;
20+
// CIR: %[[#PTR1:]] = cir.load{{.*}} %{{[0-9]+}} : !cir.ptr<!cir.ptr<!s32
21+
// CIR-NEXT: %[[#CAST:]] = cir.cast(address_space, %[[#PTR1]] : !cir.ptr<!s32i, addrspace(target<1>)>), !cir.ptr<!s32i, addrspace(target<2>)>
22+
// CIR-NEXT: cir.store{{.*}} %[[#CAST]], %{{[0-9]+}} : !cir.ptr<!s32i, addrspace(target<2>)>, !cir.ptr<!cir.ptr<!s32i, addrspace(target<2>)>>
23+
24+
// LLVM: %[[#PTR1:]] = load ptr addrspace(1), ptr %{{[0-9]+}}, align 8
25+
// LLVM-NEXT: %[[#CAST:]] = addrspacecast ptr addrspace(1) %[[#PTR1]] to ptr addrspace(2)
26+
// LLVM-NEXT: store ptr addrspace(2) %[[#CAST]], ptr %{{[0-9]+}}, align 8
27+
28+
// OGCG: %{{.*}} = load ptr addrspace(1), ptr %{{.*}}, align 8
29+
// OGCG-NEXT: %{{.*}} = addrspacecast ptr addrspace(1) %{{.*}} to ptr addrspace(2)
30+
// OGCG-NEXT: store ptr addrspace(2) %{{.*}}, ptr %{{.*}}, align 8
31+
}
32+
33+
// CIR: cir.func dso_local @{{.*test_ref.*}}
34+
// LLVM: define dso_local void @{{.*test_ref.*}}
35+
// OGCG: define dso_local void @{{.*test_ref.*}}
36+
void test_ref() {
37+
pi1_t ptr;
38+
ri1_t ref1 = *ptr;
39+
ri2_t ref2 = (ri2_t)ref1;
40+
// CIR: %[[#DEREF:]] = cir.load deref{{.*}} %{{[0-9]+}} : !cir.ptr<!cir.ptr<!s32i, addrspace(target<1>)>>, !cir.ptr<!s32i, addrspace(target<1>)>
41+
// CIR-NEXT: cir.store{{.*}} %[[#DEREF]], %[[#ALLOCAREF1:]] : !cir.ptr<!s32i, addrspace(target<1>)>, !cir.ptr<!cir.ptr<!s32i, addrspace(target<1>)>>
42+
// CIR-NEXT: %[[#REF1:]] = cir.load{{.*}} %[[#ALLOCAREF1]] : !cir.ptr<!cir.ptr<!s32i, addrspace(target<1>)>>, !cir.ptr<!s32i, addrspace(target<1>)>
43+
// CIR-NEXT: %[[#CAST:]] = cir.cast(address_space, %[[#REF1]] : !cir.ptr<!s32i, addrspace(target<1>)>), !cir.ptr<!s32i, addrspace(target<2>)>
44+
// CIR-NEXT: cir.store{{.*}} %[[#CAST]], %{{[0-9]+}} : !cir.ptr<!s32i, addrspace(target<2>)>, !cir.ptr<!cir.ptr<!s32i, addrspace(target<2>)>>
45+
46+
// LLVM: %[[#DEREF:]] = load ptr addrspace(1), ptr %{{[0-9]+}}, align 8
47+
// LLVM-NEXT: store ptr addrspace(1) %[[#DEREF]], ptr %[[#ALLOCAREF1:]], align 8
48+
// LLVM-NEXT: %[[#REF1:]] = load ptr addrspace(1), ptr %[[#ALLOCAREF1]], align 8
49+
// LLVM-NEXT: %[[#CAST:]] = addrspacecast ptr addrspace(1) %[[#REF1]] to ptr addrspace(2)
50+
// LLVM-NEXT: store ptr addrspace(2) %[[#CAST]], ptr %{{[0-9]+}}, align 8
51+
52+
// OGCG: %{{.*}} = load ptr addrspace(1), ptr %{{.*}}, align 8
53+
// OGCG-NEXT: store ptr addrspace(1) %{{.*}}, ptr %{{.*}}, align 8
54+
// OGCG-NEXT: %{{.*}} = load ptr addrspace(1), ptr %{{.*}}, align 8
55+
// OGCG-NEXT: %{{.*}} = addrspacecast ptr addrspace(1) %{{.*}} to ptr addrspace(2)
56+
// OGCG-NEXT: store ptr addrspace(2) %{{.*}}, ptr %{{.*}}, align 8
57+
}
58+
59+
// CIR: cir.func dso_local @{{.*test_nullptr.*}}
60+
// LLVM: define dso_local void @{{.*test_nullptr.*}}
61+
// OGCG: define dso_local void @{{.*test_nullptr.*}}
62+
void test_nullptr() {
63+
constexpr pi1_t null1 = nullptr;
64+
pi2_t ptr = (pi2_t)null1;
65+
// CIR: %[[#NULL1:]] = cir.const #cir.ptr<null> : !cir.ptr<!s32i, addrspace(target<1>)>
66+
// CIR-NEXT: cir.store{{.*}} %[[#NULL1]], %{{[0-9]+}} : !cir.ptr<!s32i, addrspace(target<1>)>, !cir.ptr<!cir.ptr<!s32i, addrspace(target<1>)>>
67+
// CIR-NEXT: %[[#NULL2:]] = cir.const #cir.ptr<null> : !cir.ptr<!s32i, addrspace(target<2>)>
68+
// CIR-NEXT: cir.store{{.*}} %[[#NULL2]], %{{[0-9]+}} : !cir.ptr<!s32i, addrspace(target<2>)>, !cir.ptr<!cir.ptr<!s32i, addrspace(target<2>)>>
69+
70+
// LLVM: store ptr addrspace(1) null, ptr %{{[0-9]+}}, align 8
71+
// LLVM-NEXT: store ptr addrspace(2) null, ptr %{{[0-9]+}}, align 8
72+
73+
// OGCG: store ptr addrspace(1) null, ptr %{{.*}}, align 8
74+
// OGCG-NEXT: store ptr addrspace(2) null, ptr %{{.*}}, align 8
75+
}
76+
77+
void test_side_effect(pi1_t b) {
78+
pi2_t p = (pi2_t)(*b++, (int*)0);
79+
// CIR: %{{[0-9]+}} = cir.ptr_stride(%{{[0-9]+}} : !cir.ptr<!s32i, addrspace(target<1>)>, %{{[0-9]+}} : !s32i), !cir.ptr<!s32i, addrspace(target<1>)>
80+
// CIR: %[[#CAST:]] = cir.const #cir.ptr<null> : !cir.ptr<!s32i, addrspace(target<2>)>
81+
// CIR-NEXT: cir.store{{.*}} %[[#CAST]], %{{[0-9]+}} : !cir.ptr<!s32i, addrspace(target<2>)>, !cir.ptr<!cir.ptr<!s32i, addrspace(target<2>)>>
82+
83+
// LLVM: %{{[0-9]+}} = getelementptr i32, ptr addrspace(1) %{{[0-9]+}}, i64 1
84+
// LLVM: store ptr addrspace(2) null, ptr %{{[0-9]+}}, align 8
85+
86+
// OGCG: %{{.*}} = getelementptr{{.*}} i32, ptr addrspace(1) %{{.*}}, i32 1
87+
// OGCG: store ptr addrspace(2) null, ptr %{{.*}}, align 8
88+
89+
}

0 commit comments

Comments
 (0)