Author: David Rivera Date: 2026-10-01T20:52:11-04:00 New Revision: 6c1d1068a26d789f0a7d0a4b40dc77189a0042ab
URL: https://github.com/llvm/llvm-project/commit/6c1d1068a26d789f0a7d0a4b40dc77189a0042ab DIFF: https://github.com/llvm/llvm-project/commit/6c1d1068a26d789f0a7d0a4b40dc77189a0042ab.diff LOG: [CIR] Propagate the record address space to get_member (#226650) Addresses: https://github.com/llvm/llvm-project/issues/226629 A member lives in its record's address space, but a few `get_member` builders always produced a default-AS pointer. On SPIR-V that's private, so a SYCL kernel was reading its captured pointer through a private pointer. This patch takes the AS from the base and adds a verifier check so we catch any stragglers. Assisted-by: Claude / Opus 5.5 Added: clang/test/CIR/CodeGen/get-member-addrspace.cpp clang/test/CIR/CodeGenHIP/inline-asm-multi-output-addrspace.hip Modified: clang/lib/CIR/CodeGen/CIRGenAsm.cpp clang/lib/CIR/CodeGen/CIRGenBuilder.h clang/lib/CIR/CodeGen/CIRGenBuiltinNVPTX.cpp clang/lib/CIR/CodeGen/CIRGenExpr.cpp clang/lib/CIR/Dialect/IR/CIRDialect.cpp clang/test/CIR/CodeGenSYCL/kernel-caller-address-space.cpp clang/test/CIR/IR/invalid-struct.cir Removed: ################################################################################ diff --git a/clang/lib/CIR/CodeGen/CIRGenAsm.cpp b/clang/lib/CIR/CodeGen/CIRGenAsm.cpp index cfc5cb52584ae..31b5ff07ecf64 100644 --- a/clang/lib/CIR/CodeGen/CIRGenAsm.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenAsm.cpp @@ -592,7 +592,9 @@ mlir::LogicalResult CIRGenFunction::emitAsmStmt(const AsmStmt &s) { builder.createStore(loc, result, addr); for (unsigned i = 0, e = resultRegTypes.size(); i != e; ++i) { - cir::PointerType typ = builder.getPointerTo(resultRegTypes[i]); + cir::PointerType typ = builder.getPointerTo( + resultRegTypes[i], + mlir::cast<cir::PointerType>(dest.getType()).getAddrSpace()); cir::GetMemberOp ptr = builder.createGetMember(loc, typ, dest, "", i); cir::LoadOp tmp = builder.createLoad(loc, Address(ptr, alignment)); regResults.push_back(tmp); diff --git a/clang/lib/CIR/CodeGen/CIRGenBuilder.h b/clang/lib/CIR/CodeGen/CIRGenBuilder.h index ccd4ed403e3b6..d224feb83b03d 100644 --- a/clang/lib/CIR/CodeGen/CIRGenBuilder.h +++ b/clang/lib/CIR/CodeGen/CIRGenBuilder.h @@ -623,7 +623,7 @@ class CIRGenBuilderTy : public cir::CIRBaseBuilderTy { assert(index < recordTy.getMembers().size() && "member index out of bounds"); mlir::Type memberTy = recordTy.getMembers()[index]; - mlir::Type memberPtrTy = getPointerTo(memberTy); + mlir::Type memberPtrTy = getPointerTo(memberTy, base.getAddressSpace()); auto moduleOp = getInsertionBlock()->getParentOp()->getParentOfType<mlir::ModuleOp>(); diff --git a/clang/lib/CIR/CodeGen/CIRGenBuiltinNVPTX.cpp b/clang/lib/CIR/CodeGen/CIRGenBuiltinNVPTX.cpp index b81faf65414c1..5b3618c137fb7 100644 --- a/clang/lib/CIR/CodeGen/CIRGenBuiltinNVPTX.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenBuiltinNVPTX.cpp @@ -1042,7 +1042,8 @@ static mlir::Value packArgsIntoNVPTXFormatBuffer(CIRGenFunction &cgf, for (auto [i, arg] : llvm::enumerate(llvm::drop_begin(args))) { mlir::Value member = builder.createGetMember( - loc, cir::PointerType::get(argTypes[i]), alloca, /*name=*/"", + loc, cir::PointerType::get(argTypes[i], allocaAddr.getAddressSpace()), + alloca, /*name=*/"", /*index=*/i); auto abiAlign = clang::CharUnits::fromQuantity( dataLayout.getABITypeAlign(argTypes[i]).value()); diff --git a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp index 3ce1602a83c4b..08901b30084bd 100644 --- a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp @@ -92,12 +92,14 @@ Address CIRGenFunction::emitAddrOfFieldStorage(Address base, // For unions, all fields map to index 0, so we use the field's declared type // directly instead of looking up the member type from the layout. mlir::Type fieldType = convertType(field->getType()); - auto fieldPtr = cir::PointerType::get(fieldType); + // A member lives in the same address space as its record. + mlir::ptr::MemorySpaceAttrInterface addrSpace = base.getAddressSpace(); + auto fieldPtr = cir::PointerType::get(fieldType, addrSpace); bool needsBitcast = false; if (!rec->isUnion() && field->isPotentiallyOverlapping()) { mlir::Type memberType = layout.getCIRType().getMembers()[idx]; - fieldPtr = cir::PointerType::get(memberType); + fieldPtr = cir::PointerType::get(memberType, addrSpace); needsBitcast = true; } @@ -550,7 +552,8 @@ Address CIRGenFunction::getAddrOfBitFieldStorage(LValue base, mlir::Type fieldType, unsigned index) { mlir::Location loc = getLoc(field->getLocation()); - cir::PointerType fieldPtr = cir::PointerType::get(fieldType); + cir::PointerType fieldPtr = + cir::PointerType::get(fieldType, base.getAddress().getAddressSpace()); auto rec = cast<cir::RecordType>(base.getAddress().getElementType()); cir::GetMemberOp sea = getBuilder().createGetMember( loc, fieldPtr, base.getPointer(), field->getName(), diff --git a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp index c4ed6697b869f..fd6cbcc717b43 100644 --- a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp +++ b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp @@ -3961,6 +3961,9 @@ LogicalResult cir::GetMemberOp::verify() { if (pointeeTy != getType().getPointee()) return emitError() << "member type mismatch"; + if (getAddrTy().getAddrSpace() != getType().getAddrSpace()) + return emitError() << "address space mismatch"; + return mlir::success(); } diff --git a/clang/test/CIR/CodeGen/get-member-addrspace.cpp b/clang/test/CIR/CodeGen/get-member-addrspace.cpp new file mode 100644 index 0000000000000..6a8c1605f4cc2 --- /dev/null +++ b/clang/test/CIR/CodeGen/get-member-addrspace.cpp @@ -0,0 +1,71 @@ +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o %t.cir +// RUN: FileCheck --input-file=%t.cir %s -check-prefix=CIR +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-llvm %s -o %t-cir.ll +// RUN: FileCheck --input-file=%t-cir.ll %s -check-prefix=LLVM +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o %t.ll +// RUN: FileCheck --input-file=%t.ll %s -check-prefix=OGCG + +// A member is stored inside its record, so the pointer to it is in the address +// space of the record. + +#define AS1 __attribute__((address_space(1))) + +struct S { int a; int b : 3; }; +union U { int i; float f; }; +struct B { B(); int i; char c; }; +struct O { [[no_unique_address]] B b; char d; }; + +int field(AS1 S *s) { return s->a; } + +// CIR-LABEL: cir.func {{.*}}@_Z5fieldPU3AS11S +// CIR: cir.get_member %{{.*}}[0] {name = "a"} : !cir.ptr<!rec_S, target_address_space(1)> -> !cir.ptr<!s32i, target_address_space(1)> + +// LLVM-LABEL: define {{.*}}@_Z5fieldPU3AS11S +// LLVM: %[[A:.*]] = getelementptr inbounds nuw %struct.S, ptr addrspace(1) %{{.*}}, i32 0, i32 0 +// LLVM: load i32, ptr addrspace(1) %[[A]] + +// OGCG-LABEL: define {{.*}}@_Z5fieldPU3AS11S +// OGCG: %[[A:.*]] = getelementptr inbounds nuw %struct.S, ptr addrspace(1) %{{.*}}, i32 0, i32 0 +// OGCG: load i32, ptr addrspace(1) %[[A]] + +void bitfield(AS1 S *s) { s->b = 2; } + +// CIR-LABEL: cir.func {{.*}}@_Z8bitfieldPU3AS11S +// CIR: cir.get_member %{{.*}}[1] {name = "b"} : !cir.ptr<!rec_S, target_address_space(1)> -> !cir.ptr<!u8i, target_address_space(1)> + +// LLVM-LABEL: define {{.*}}@_Z8bitfieldPU3AS11S +// LLVM: %[[B:.*]] = getelementptr inbounds nuw %struct.S, ptr addrspace(1) %{{.*}}, i32 0, i32 1 +// LLVM: store i8 %{{.*}}, ptr addrspace(1) %[[B]] + +// OGCG-LABEL: define {{.*}}@_Z8bitfieldPU3AS11S +// OGCG: %[[B:.*]] = getelementptr inbounds nuw %struct.S, ptr addrspace(1) %{{.*}}, i32 0, i32 1 +// OGCG: store i8 %{{.*}}, ptr addrspace(1) %[[B]] + +float union_member(AS1 U *u) { return u->f; } + +// CIR-LABEL: cir.func {{.*}}@_Z12union_memberPU3AS11U +// CIR: cir.get_member %{{.*}}[1] {name = "f"} : !cir.ptr<!rec_U, target_address_space(1)> -> !cir.ptr<!cir.float, target_address_space(1)> + +// LLVM-LABEL: define {{.*}}@_Z12union_memberPU3AS11U +// LLVM: %[[U:.*]] = load ptr addrspace(1), ptr +// LLVM: load float, ptr addrspace(1) %[[U]] + +// OGCG-LABEL: define {{.*}}@_Z12union_memberPU3AS11U +// OGCG: %[[U:.*]] = load ptr addrspace(1), ptr +// OGCG: load float, ptr addrspace(1) %[[U]] + +int overlapping(AS1 O *o) { return o->b.i; } + +// CIR-LABEL: cir.func {{.*}}@_Z11overlappingPU3AS11O +// CIR: %[[BASE:.*]] = cir.get_member %{{.*}}[0] {name = "b"} : !cir.ptr<!rec_O, target_address_space(1)> -> !cir.ptr<!rec_B2Ebase, target_address_space(1)> +// CIR: cir.cast bitcast %[[BASE]] : !cir.ptr<!rec_B2Ebase, target_address_space(1)> -> !cir.ptr<!rec_B, target_address_space(1)> + +// LLVM-LABEL: define {{.*}}@_Z11overlappingPU3AS11O +// LLVM: %[[OB:.*]] = getelementptr inbounds nuw %struct.O, ptr addrspace(1) %{{.*}}, i32 0, i32 0 +// LLVM: %[[I:.*]] = getelementptr inbounds nuw %struct.B, ptr addrspace(1) %[[OB]], i32 0, i32 0 +// LLVM: load i32, ptr addrspace(1) %[[I]] + +// OGCG-LABEL: define {{.*}}@_Z11overlappingPU3AS11O +// OGCG: %[[OB:.*]] = getelementptr inbounds nuw %struct.O, ptr addrspace(1) %{{.*}}, i32 0, i32 0 +// OGCG: %[[I:.*]] = getelementptr inbounds nuw %struct.B, ptr addrspace(1) %[[OB]], i32 0, i32 0 +// OGCG: load i32, ptr addrspace(1) %[[I]] diff --git a/clang/test/CIR/CodeGenHIP/inline-asm-multi-output-addrspace.hip b/clang/test/CIR/CodeGenHIP/inline-asm-multi-output-addrspace.hip new file mode 100644 index 0000000000000..4f15e407ef354 --- /dev/null +++ b/clang/test/CIR/CodeGenHIP/inline-asm-multi-output-addrspace.hip @@ -0,0 +1,24 @@ +// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -x hip -fcuda-is-device \ +// RUN: -fclangir -emit-cir %s -o %t.cir +// RUN: FileCheck --input-file=%t.cir %s -check-prefix=CIR +// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -x hip -fcuda-is-device \ +// RUN: -fclangir -emit-llvm %s -o %t.ll +// RUN: FileCheck --input-file=%t.ll %s -check-prefix=LLVM + +// The results of a multi-output asm are read back from a private (AS 5) +// temporary, so the member pointers must stay in AS 5. + +__attribute__((device)) int two_outputs() { + int a, b; + asm("v_mov_b32 %0, 1\n v_mov_b32 %1, 2" : "=v"(a), "=v"(b)); + return a + b; +} + +// CIR-LABEL: cir.func {{.*}}@_Z11two_outputsv +// CIR: %[[RES:.*]] = cir.alloca "__asm_result" {{.*}} : !cir.ptr<!rec_anon_struct, target_address_space(5)> +// CIR: cir.get_member %[[RES]][0] {name = ""} : !cir.ptr<!rec_anon_struct, target_address_space(5)> -> !cir.ptr<!s32i, target_address_space(5)> +// CIR: cir.get_member %[[RES]][1] {name = ""} : !cir.ptr<!rec_anon_struct, target_address_space(5)> -> !cir.ptr<!s32i, target_address_space(5)> + +// LLVM-LABEL: define {{.*}}@_Z11two_outputsv +// LLVM: getelementptr inbounds nuw { i32, i32 }, ptr addrspace(5) %{{.*}}, i32 0, i32 0 +// LLVM: getelementptr inbounds nuw { i32, i32 }, ptr addrspace(5) %{{.*}}, i32 0, i32 1 diff --git a/clang/test/CIR/CodeGenSYCL/kernel-caller-address-space.cpp b/clang/test/CIR/CodeGenSYCL/kernel-caller-address-space.cpp index 306917a46502b..cb4a696264430 100644 --- a/clang/test/CIR/CodeGenSYCL/kernel-caller-address-space.cpp +++ b/clang/test/CIR/CodeGenSYCL/kernel-caller-address-space.cpp @@ -38,7 +38,7 @@ void test(int *p) { // The kernel functor's operator() reads the captured pointer, which is a // generic-address-space pointer, and stores through it. // CIR-LABEL: cir.func {{.*}}@_ZZ4testPiENKUlvE_clEv -// CIR: cir.get_member {{.*}} -> !cir.ptr<!cir.ptr<!s32i, target_address_space(4)>> +// CIR: cir.get_member {{.*}} -> !cir.ptr<!cir.ptr<!s32i, target_address_space(4)>, target_address_space(4)> // CIR: cir.store {{.*}} : !s32i, !cir.ptr<!s32i, target_address_space(4)> // The captured pointer field and the store through it use address space 4, diff --git a/clang/test/CIR/IR/invalid-struct.cir b/clang/test/CIR/IR/invalid-struct.cir index c72f142ce03c1..713434dee6f36 100644 --- a/clang/test/CIR/IR/invalid-struct.cir +++ b/clang/test/CIR/IR/invalid-struct.cir @@ -117,3 +117,17 @@ module { cir.return } } + +// ----- + +!u8i = !cir.int<u, 8> +!u32i = !cir.int<u, 32> +!rec_S = !cir.struct<"S" {data !u8i, data !u32i}> + +module { + cir.func @struct_get_member_address_space_mismatch(%arg0: !cir.ptr<!rec_S, target_address_space(1)>) { + // expected-error @below {{address space mismatch}} + %0 = cir.get_member %arg0[1] {name = "b"} : !cir.ptr<!rec_S, target_address_space(1)> -> !cir.ptr<!u32i> + cir.return + } +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
