Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions clang/include/clang/CIR/Dialect/IR/CIRAttrs.td
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,46 @@ def CIR_GlobalViewAttr : CIR_Attr<"GlobalView", "global_view", [
}];
}

//===----------------------------------------------------------------------===//
// VTableAttr
//===----------------------------------------------------------------------===//

def VTableAttr : CIR_Attr<"VTable", "vtable", [TypedAttrInterface]> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def VTableAttr : CIR_Attr<"VTable", "vtable", [TypedAttrInterface]> {
def CIR_VTableAttr : CIR_Attr<"VTable", "vtable", [TypedAttrInterface]> {

let summary = "Represents a C++ vtable";
let description = [{
Wraps a #cir.const_record containing vtable data.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get how this wraps record, since it wraps vtable array?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can potentially wrap a record containing multiple vtable arrays. In the simple case, we create an anonymous record that has a single array element, which looks like this:

cir.global linkonce_odr @_ZTV6Mother =
  #cir.vtable<{
    #cir.const_array<[
      #cir.ptr<null> : !cir.ptr<!u8i>,
      #cir.global_view<@_ZTI6Mother> : !cir.ptr<!u8i>,
      #cir.global_view<@_ZN6Mother9MotherFooEv> : !cir.ptr<!u8i>,
      #cir.global_view<@_ZN6Mother10MotherFoo2Ev> : !cir.ptr<!u8i>
    ]> : !cir.array<!cir.ptr<!u8i> x 4>
  }> : !rec_anon_struct2 {alignment = 8 : i64 }

But in cases involving multiple inheritence, there can be multiple vtables within the record, like this:

cir.global linkonce_odr @_ZTV5Child =
  #cir.vtable<{
    #cir.const_array<[
      #cir.ptr<null> : !cir.ptr<!u8i>,
      #cir.global_view<@_ZTI5Child> : !cir.ptr<!u8i>,
      #cir.global_view<@_ZN5Child9MotherFooEv> : !cir.ptr<!u8i>,
      #cir.global_view<@_ZN6Mother10MotherFoo2Ev> : !cir.ptr<!u8i>
    ]> : !cir.array<!cir.ptr<!u8i> x 4>,
    #cir.const_array<[
      #cir.ptr<-8 : i64> : !cir.ptr<!u8i>,
      #cir.global_view<@_ZTI5Child> : !cir.ptr<!u8i>,
      #cir.global_view<@_ZN6Father9FatherFooEv> : !cir.ptr<!u8i>
    ]> : !cir.array<!cir.ptr<!u8i> x 3>
  }> : !rec_anon_struct4 {alignment = 8 : i64}

The index attribute in the address_point selects which of the wrapped vtables is being referenced:

` %6 = cir.vtable.address_point(@_ZTV5Child, address_point = <index = 1, offset = 2>) : !cir.vptr

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah makes sense, thanks for explanation. With example this will be clearer :)


Example:
```
cir.global linkonce_odr @_ZTV1B = #cir.vtable<<
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There seem to be extra brackets in this example, which may be making it harder to understand.

{#cir.const_array<[#cir.null : !cir.ptr<i8>,
#cir.global_view<@_ZTI1B> : !cir.ptr<i8>,
#cir.global_view<@_ZN1BD1Ev> : !cir.ptr<i8>,
#cir.global_view<@_ZN1BD0Ev> : !cir.ptr<i8>,
#cir.global_view<@_ZNK1A5quackEv> : !cir.ptr<i8>]>
: !cir.array<!cir.ptr<i8> x 5>}>>
: !cir.record<"", !cir.array<!cir.ptr<i8> x 5>>
```
}];

// `vtable_data` is a const record with one element, containing an array of
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, it is not const_record but ArrayAttr?
Wasn't the intent to say vtable_data is a single element of vtable const record?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned above, there can be multiple arrays. I'll update this comment to reflect that and add an example with two arrays.

// vtable information.
let parameters = (ins AttributeSelfTypeParameter<"">:$type,
"mlir::ArrayAttr":$vtable_data);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let parameters = (ins AttributeSelfTypeParameter<"">:$type,
"mlir::ArrayAttr":$vtable_data);
let parameters = (ins
AttributeSelfTypeParameter<"">:$type,
"mlir::ArrayAttr":$vtable_data
);


let builders = [
AttrBuilderWithInferredContext<(ins "mlir::Type":$type,
"mlir::ArrayAttr":$vtable_data), [{
return $_get(type.getContext(), type, vtable_data);
}]>
];

let genVerifyDecl = 1;
let assemblyFormat = [{
`<` custom<RecordMembers>($vtable_data) `>`
}];
}

//===----------------------------------------------------------------------===//
// ConstComplexAttr
//===----------------------------------------------------------------------===//
Expand Down
38 changes: 38 additions & 0 deletions clang/lib/CIR/Dialect/IR/CIRAttrs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,44 @@ cir::ConstVectorAttr::verify(function_ref<InFlightDiagnostic()> emitError,
return elementTypeCheck;
}

//===----------------------------------------------------------------------===//
// CIR VTableAttr
//===----------------------------------------------------------------------===//

LogicalResult cir::VTableAttr::verify(
llvm::function_ref<mlir::InFlightDiagnostic()> emitError, mlir::Type type,
mlir::ArrayAttr vtableData) {
auto sTy = mlir::dyn_cast_if_present<cir::RecordType>(type);
if (!sTy)
return emitError() << "expected !cir.record type result";
if (sTy.getMembers().empty() || vtableData.empty())
return emitError() << "expected record type with one or more subtype";

for (size_t i = 0; i < sTy.getMembers().size(); ++i) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be only one member, so the loop is a bit misleading here.

Cannot we just assert that size == 1 and that the single element is the expected array?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it is completely redundant check as cir::ConstRecordAttr::verify checks this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought the same thing when I was moving this over and had changed it in the way you suggest before finding the multi-vtable example (clang/test/CIR/CodeGen/multi-vtable.cpp in the incubator). It didn't occur to me until just now that, like you, I probably thought that because of what the comments in the attribute declaration said.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we use a range based for loop here, or if we need the index, use llvm::enumerate instead? We’re currently just using i for indexing

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good suggestion. Thanks. It also led me to notice something I had missed.

auto constArrayAttr = mlir::dyn_cast<cir::ConstArrayAttr>(vtableData[i]);
if (!constArrayAttr)
return emitError() << "expected constant array subtype";

if (cir::ConstRecordAttr::verify(emitError, type, vtableData).failed())
return failure();

LogicalResult eltTypeCheck = success();
auto arrayElts = mlir::cast<ArrayAttr>(constArrayAttr.getElts());
arrayElts.walkImmediateSubElements(
[&](Attribute attr) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[&](Attribute attr) {
[&](mlir::Attribute attr) {

if (mlir::isa<ConstPtrAttr, GlobalViewAttr>(attr))
return;

eltTypeCheck = emitError()
<< "expected GlobalViewAttr or ConstPtrAttr";
},
[&](Type type) {});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[&](Type type) {});
[&](mlir::Type type) {});

if (eltTypeCheck.failed())
return eltTypeCheck;
}
return success();
}

//===----------------------------------------------------------------------===//
// CIR Dialect
//===----------------------------------------------------------------------===//
Expand Down
3 changes: 2 additions & 1 deletion clang/lib/CIR/Dialect/IR/CIRDialect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,8 @@ static LogicalResult checkConstantTypes(mlir::Operation *op, mlir::Type opType,

if (mlir::isa<cir::ConstArrayAttr, cir::ConstVectorAttr,
cir::ConstComplexAttr, cir::ConstRecordAttr,
cir::GlobalViewAttr, cir::PoisonAttr>(attrType))
cir::GlobalViewAttr, cir::PoisonAttr, cir::VTableAttr>(
attrType))
return success();

assert(isa<TypedAttr>(attrType) && "What else could we be looking at here?");
Expand Down
50 changes: 49 additions & 1 deletion clang/test/CIR/IR/invalid-vtable.cir
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// RUN: cir-opt %s -verify-diagnostics
// RUN: cir-opt %s -verify-diagnostics -split-input-file

!s8i = !cir.int<s, 8>
!u32i = !cir.int<u, 32>
Expand All @@ -7,3 +7,51 @@ cir.func @reference_unknown_vtable() {
%0 = cir.vtable.address_point(@some_vtable, address_point = <index = 0, offset = 2>) : !cir.vptr
cir.return
}

// -----

!rec_S = !cir.record<struct "S" {!cir.vptr}>
!u8i = !cir.int<u, 8>
!rec_anon_struct = !cir.record<struct {!cir.array<!cir.ptr<!u8i> x 4>}>
module {
// expected-error @below {{expected !cir.record type result}}
cir.global external @_ZTV1S = #cir.vtable<{#cir.const_array<[#cir.ptr<null> : !cir.ptr<!u8i>, #cir.ptr<null> : !cir.ptr<!u8i>, #cir.global_view<@_ZN1S3keyEv> : !cir.ptr<!u8i>, #cir.global_view<@_ZN1S6nonKeyEv> : !cir.ptr<!u8i>]> : !cir.array<!cir.ptr<!u8i> x 4>}> : !cir.ptr<!rec_anon_struct>
cir.func private dso_local @_ZN1S3keyEv(%arg0: !cir.ptr<!rec_S>)
cir.func private dso_local @_ZN1S6nonKeyEv(%arg0: !cir.ptr<!rec_S>)
}

// -----

!rec_S = !cir.record<struct "S" {!cir.vptr}>
!u8i = !cir.int<u, 8>
!rec_anon_struct = !cir.record<struct {}>
module {
// expected-error @below {{expected record type with one or more subtype}}
cir.global external @_ZTV1S = #cir.vtable<{}> : !rec_anon_struct {alignment = 8 : i64}
cir.func private dso_local @_ZN1S3keyEv(%arg0: !cir.ptr<!rec_S>)
cir.func private dso_local @_ZN1S6nonKeyEv(%arg0: !cir.ptr<!rec_S>)
}

// -----

!rec_S = !cir.record<struct "S" {!cir.vptr}>
!u8i = !cir.int<u, 8>
!rec_anon_struct = !cir.record<struct {!cir.ptr<!u8i>}>
module {
// expected-error @below {{expected constant array subtype}}
cir.global external @_ZTV1S = #cir.vtable<{#cir.ptr<null> : !cir.ptr<!u8i>}> : !rec_anon_struct {alignment = 8 : i64}
cir.func private dso_local @_ZN1S3keyEv(%arg0: !cir.ptr<!rec_S>)
cir.func private dso_local @_ZN1S6nonKeyEv(%arg0: !cir.ptr<!rec_S>)
}

// -----

!rec_S = !cir.record<struct "S" {!cir.vptr}>
!u64i = !cir.int<u, 64>
!rec_anon_struct = !cir.record<struct {!cir.array<!u64i x 4>}>
module {
// expected-error @below {{expected GlobalViewAttr or ConstPtrAttr}}
cir.global external @_ZTV1S = #cir.vtable<{#cir.const_array<[#cir.int<1> : !u64i, #cir.int<1> : !u64i, #cir.int<3> : !u64i, #cir.int<4> : !u64i]> : !cir.array<!u64i x 4>}> : !rec_anon_struct {alignment = 8 : i64}
cir.func private dso_local @_ZN1S3keyEv(%arg0: !cir.ptr<!rec_S>)
cir.func private dso_local @_ZN1S6nonKeyEv(%arg0: !cir.ptr<!rec_S>)
}
12 changes: 12 additions & 0 deletions clang/test/CIR/IR/vtable-attr.cir
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// RUN: cir-opt %s | FileCheck %s

!rec_S = !cir.record<struct "S" {!cir.vptr}>
!u8i = !cir.int<u, 8>
!rec_anon_struct = !cir.record<struct {!cir.array<!cir.ptr<!u8i> x 4>}>
module {
cir.global external @_ZTV1S = #cir.vtable<{#cir.const_array<[#cir.ptr<null> : !cir.ptr<!u8i>, #cir.ptr<null> : !cir.ptr<!u8i>, #cir.global_view<@_ZN1S3keyEv> : !cir.ptr<!u8i>, #cir.global_view<@_ZN1S6nonKeyEv> : !cir.ptr<!u8i>]> : !cir.array<!cir.ptr<!u8i> x 4>}> : !rec_anon_struct {alignment = 8 : i64}
// CHECK: cir.global external @_ZTV1S = #cir.vtable<{#cir.const_array<[#cir.ptr<null> : !cir.ptr<!u8i>, #cir.ptr<null> : !cir.ptr<!u8i>, #cir.global_view<@_ZN1S3keyEv> : !cir.ptr<!u8i>, #cir.global_view<@_ZN1S6nonKeyEv> : !cir.ptr<!u8i>]> : !cir.array<!cir.ptr<!u8i> x 4>}> : !rec_anon_struct {alignment = 8 : i64}

cir.func private dso_local @_ZN1S3keyEv(%arg0: !cir.ptr<!rec_S>)
cir.func private dso_local @_ZN1S6nonKeyEv(%arg0: !cir.ptr<!rec_S>)
}