From 6f1ce63ac19107aa95c215de78c99af4b1d7f235 Mon Sep 17 00:00:00 2001 From: gitoleg Date: Wed, 18 Oct 2023 04:20:06 +0300 Subject: [PATCH] [CIR][IR] Relax get_member verifier for incomplete types (#269) This is a suggestion to relax the existing verification even more than we did it in PR #257. Here we also skip verification if a field on the given index is also of incomplete type - and we can not compare it with the result type of the operation. Now the next code fails with type mismatch error: ``` typedef struct Node { struct Node* next; } NodeStru; void foo(NodeStru* a) { a->next = 0; } ``` because the result type is kind of full and the type of field is not (for the reasons discussed in #256). Basically, the problem is in the `GetMemberOp` result type generated as following (via `CIRGenTypes::convertType`) `!cir.ptr>} #cir.record.decl.ast>>` where the field type at index differs from the record type - compare with `!cir.ptr>` We just slightly relax the previous solution in #257 - and the compilation won't fail in the case of recursive types. Well, if there are some other thoughts? --- clang/lib/CIR/Dialect/IR/CIRDialect.cpp | 18 +++++++++++++++--- clang/test/CIR/CodeGen/struct.c | 13 +++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp index 5763971fa49e..99d7381c577c 100644 --- a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp +++ b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp @@ -2412,6 +2412,14 @@ LogicalResult MemCpyOp::verify() { return mlir::success(); } +static bool isIncompleteType(mlir::Type typ) { + if (auto ptr = typ.dyn_cast()) + return isIncompleteType(ptr.getPointee()); + else if (auto rec = typ.dyn_cast()) + return !rec.getBody(); + return false; +} + //===----------------------------------------------------------------------===// // GetMemberOp Definitions //===----------------------------------------------------------------------===// @@ -2424,7 +2432,7 @@ LogicalResult GetMemberOp::verify() { // FIXME: currently we bypass typechecking of incomplete types due to errors // in the codegen process. This should be removed once the codegen is fixed. - if (!recordTy.getBody()) + if (isIncompleteType(recordTy)) return mlir::success(); if (recordTy.getMembers().size() <= getIndex()) @@ -2432,8 +2440,12 @@ LogicalResult GetMemberOp::verify() { // FIXME(cir): member type check is disabled for classes as the codegen for // these still need to be patched. - if (!recordTy.isClass() && - recordTy.getMembers()[getIndex()] != getResultTy().getPointee()) + // Also we bypass the typechecking for the fields of incomplete types. + bool shouldSkipMemberTypeMismatch = + recordTy.isClass() || isIncompleteType(recordTy.getMembers()[getIndex()]); + + if (!shouldSkipMemberTypeMismatch + && recordTy.getMembers()[getIndex()] != getResultTy().getPointee()) return emitError() << "member type mismatch"; return mlir::success(); diff --git a/clang/test/CIR/CodeGen/struct.c b/clang/test/CIR/CodeGen/struct.c index 5df517c3cbcb..47680ab4486d 100644 --- a/clang/test/CIR/CodeGen/struct.c +++ b/clang/test/CIR/CodeGen/struct.c @@ -12,11 +12,18 @@ struct Foo { struct Bar z; }; +// Recursive type +typedef struct Node { + struct Node* next; +} NodeStru; + void baz(void) { struct Bar b; struct Foo f; } +// CHECK-DAG: !ty_22Node22 = !cir.struct +// CHECK-DAG: !ty_22Node221 = !cir.struct} #cir.record.decl.ast> // CHECK-DAG: !ty_22Bar22 = !cir.struct // CHECK-DAG: !ty_22Foo22 = !cir.struct // CHECK-DAG: module {{.*}} { @@ -78,3 +85,9 @@ struct Bar shouldGenerateAndAccessStructArrays(void) { // CHECK-DAG: %[[#DARR:]] = cir.cast(array_to_ptrdecay, %{{.+}} : !cir.ptr>), !cir.ptr // CHECK-DAG: %[[#ELT:]] = cir.ptr_stride(%[[#DARR]] : !cir.ptr, %[[#STRIDE]] : !s32i), !cir.ptr // CHECK-DAG: cir.copy %[[#ELT]] to %{{.+}} : !cir.ptr + +// CHECK-DAG: cir.func @useRecursiveType +// CHECK-DAG: cir.get_member {{%.}}[0] {name = "next"} : !cir.ptr -> !cir.ptr> +void useRecursiveType(NodeStru* a) { + a->next = 0; +}