Skip to content

Commit

Permalink
[CIR][IR] Relax get_member verifier for incomplete types (#269)
Browse files Browse the repository at this point in the history
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.struct<struct "Node" {!cir.ptr<!cir.struct<struct "Node"
incomplete #cir.record.decl.ast>>} #cir.record.decl.ast>>`

where the field type at index differs from the record type - compare
with
 `!cir.ptr<!cir.struct<struct "Node" incomplete #cir.record.decl.ast>>`

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?
  • Loading branch information
gitoleg authored and lanza committed Jan 29, 2024
1 parent 9a7d8a3 commit 6f1ce63
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 3 deletions.
18 changes: 15 additions & 3 deletions clang/lib/CIR/Dialect/IR/CIRDialect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2412,6 +2412,14 @@ LogicalResult MemCpyOp::verify() {
return mlir::success();
}

static bool isIncompleteType(mlir::Type typ) {
if (auto ptr = typ.dyn_cast<PointerType>())
return isIncompleteType(ptr.getPointee());
else if (auto rec = typ.dyn_cast<StructType>())
return !rec.getBody();
return false;
}

//===----------------------------------------------------------------------===//
// GetMemberOp Definitions
//===----------------------------------------------------------------------===//
Expand All @@ -2424,16 +2432,20 @@ 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())
return emitError() << "member index out of bounds";

// 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();
Expand Down
13 changes: 13 additions & 0 deletions clang/test/CIR/CodeGen/struct.c
Original file line number Diff line number Diff line change
Expand Up @@ -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<struct "Node" incomplete #cir.record.decl.ast>
// CHECK-DAG: !ty_22Node221 = !cir.struct<struct "Node" {!cir.ptr<!ty_22Node22>} #cir.record.decl.ast>
// CHECK-DAG: !ty_22Bar22 = !cir.struct<struct "Bar" {!s32i, !s8i}>
// CHECK-DAG: !ty_22Foo22 = !cir.struct<struct "Foo" {!s32i, !s8i, !ty_22Bar22}>
// CHECK-DAG: module {{.*}} {
Expand Down Expand Up @@ -78,3 +85,9 @@ struct Bar shouldGenerateAndAccessStructArrays(void) {
// CHECK-DAG: %[[#DARR:]] = cir.cast(array_to_ptrdecay, %{{.+}} : !cir.ptr<!cir.array<!ty_22Bar22 x 1>>), !cir.ptr<!ty_22Bar22>
// CHECK-DAG: %[[#ELT:]] = cir.ptr_stride(%[[#DARR]] : !cir.ptr<!ty_22Bar22>, %[[#STRIDE]] : !s32i), !cir.ptr<!ty_22Bar22>
// CHECK-DAG: cir.copy %[[#ELT]] to %{{.+}} : !cir.ptr<!ty_22Bar22>

// CHECK-DAG: cir.func @useRecursiveType
// CHECK-DAG: cir.get_member {{%.}}[0] {name = "next"} : !cir.ptr<!ty_22Node221> -> !cir.ptr<!cir.ptr<!ty_22Node221>>
void useRecursiveType(NodeStru* a) {
a->next = 0;
}

0 comments on commit 6f1ce63

Please sign in to comment.