-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[CIR] Upstream minimal support for structure types #135105
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
3b19527
72b4b2d
2223527
5e85698
ca8c49f
3259c1a
01652d3
ae78ae4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,118 @@ | ||
| //===----------------------------------------------------------------------===// | ||
| // | ||
| // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. | ||
| // See https://llvm.org/LICENSE.txt for license information. | ||
| // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||
| // | ||
| //===----------------------------------------------------------------------===// | ||
| // | ||
| // This file contains implementation details, such as storage structures, of | ||
| // CIR dialect types. | ||
| // | ||
| //===----------------------------------------------------------------------===// | ||
| #ifndef CIR_DIALECT_IR_CIRTYPESDETAILS_H | ||
| #define CIR_DIALECT_IR_CIRTYPESDETAILS_H | ||
|
|
||
| #include "mlir/IR/BuiltinAttributes.h" | ||
| #include "mlir/Support/LogicalResult.h" | ||
| #include "clang/CIR/Dialect/IR/CIRTypes.h" | ||
| #include "llvm/ADT/Hashing.h" | ||
|
|
||
| namespace cir { | ||
| namespace detail { | ||
|
|
||
| //===----------------------------------------------------------------------===// | ||
| // CIR RecordTypeStorage | ||
| //===----------------------------------------------------------------------===// | ||
|
|
||
| /// Type storage for CIR record types. | ||
| struct RecordTypeStorage : public mlir::TypeStorage { | ||
| struct KeyTy { | ||
| llvm::ArrayRef<mlir::Type> members; | ||
| mlir::StringAttr name; | ||
| bool incomplete; | ||
| bool packed; | ||
| bool padded; | ||
| RecordType::RecordKind kind; | ||
|
|
||
| KeyTy(llvm::ArrayRef<mlir::Type> members, mlir::StringAttr name, | ||
| bool incomplete, bool packed, bool padded, | ||
| RecordType::RecordKind kind) | ||
| : members(members), name(name), incomplete(incomplete), packed(packed), | ||
| padded(padded), kind(kind) {} | ||
| }; | ||
|
|
||
| llvm::ArrayRef<mlir::Type> members; | ||
| mlir::StringAttr name; | ||
| bool incomplete; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I findmyself wishing this was
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can understand that.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would still, I think, like hte polarity here reversed.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK. I was hesitant to do that because I'm not sure how this is all plumbed together, but I'll give it a shot.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To make sure we're on the same page, are you asking for this to be changed to Do you want to change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In fact, it seems that I can't change it here without changing it all the way up to the type in the .td file. I can have it print "incomplete" only when
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would prefer all the way up. Though it is a decently strong preference, I'm willing to let this get delayed until a future patch where we discuss it with @bcardosolopes
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm more in favor of keeping it "incomplete" because it highlights the corner case/state instead of the most common case, but I agree it might be worth pushing that to another PR (since as Andy mentioned this has a deep side-effect with more changes required). |
||
| bool packed; | ||
| bool padded; | ||
| RecordType::RecordKind kind; | ||
|
|
||
| RecordTypeStorage(llvm::ArrayRef<mlir::Type> members, mlir::StringAttr name, | ||
| bool incomplete, bool packed, bool padded, | ||
| RecordType::RecordKind kind) | ||
| : members(members), name(name), incomplete(incomplete), packed(packed), | ||
| padded(padded), kind(kind) { | ||
| assert(name || !incomplete && "Incomplete records must have a name"); | ||
| } | ||
|
|
||
| KeyTy getAsKey() const { | ||
| return KeyTy(members, name, incomplete, packed, padded, kind); | ||
| } | ||
|
|
||
| bool operator==(const KeyTy &key) const { | ||
| if (name) | ||
| return (name == key.name) && (kind == key.kind); | ||
| return std::tie(members, name, incomplete, packed, padded, kind) == | ||
| std::tie(key.members, key.name, key.incomplete, key.packed, | ||
| key.padded, key.kind); | ||
| } | ||
|
|
||
| static llvm::hash_code hashKey(const KeyTy &key) { | ||
| if (key.name) | ||
| return llvm::hash_combine(key.name, key.kind); | ||
| return llvm::hash_combine(key.members, key.incomplete, key.packed, | ||
| key.padded, key.kind); | ||
| } | ||
|
|
||
| static RecordTypeStorage *construct(mlir::TypeStorageAllocator &allocator, | ||
| const KeyTy &key) { | ||
| return new (allocator.allocate<RecordTypeStorage>()) | ||
| RecordTypeStorage(allocator.copyInto(key.members), key.name, | ||
| key.incomplete, key.packed, key.padded, key.kind); | ||
| } | ||
|
|
||
| /// Mutates the members and attributes an identified record. | ||
| /// | ||
| /// Once a record is mutated, it is marked as complete, preventing further | ||
| /// mutations. Anonymous records are always complete and cannot be mutated. | ||
| /// This method does not fail if a mutation of a complete record does not | ||
| /// change the record. | ||
| llvm::LogicalResult mutate(mlir::TypeStorageAllocator &allocator, | ||
| llvm::ArrayRef<mlir::Type> members, bool packed, | ||
| bool padded) { | ||
| // Anonymous records cannot mutate. | ||
| if (!name) | ||
| return llvm::failure(); | ||
|
|
||
| // Mutation of complete records are allowed if they change nothing. | ||
| if (!incomplete) | ||
| return mlir::success((this->members == members) && | ||
| (this->packed == packed) && | ||
| (this->padded == padded)); | ||
|
|
||
| // Mutate incomplete record. | ||
| this->members = allocator.copyInto(members); | ||
| this->packed = packed; | ||
| this->padded = padded; | ||
|
|
||
| incomplete = false; | ||
| return llvm::success(); | ||
| } | ||
| }; | ||
|
|
||
| } // namespace detail | ||
| } // namespace cir | ||
|
|
||
| #endif // CIR_DIALECT_IR_CIRTYPESDETAILS_H | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What value is there in differentiating between 'class' and 'struct' ? From a language perspective, they are effectively synonyms.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure. I want to say that it has to do with understanding the intent of the source code, but as you say there is no reason that the user can't have been using them interchangeably. I just verified that if I create a
structwith a virtual method and a base class (declared withclass), the incubator still calls it astructhere..There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't mind this btw, more for clarification. The only actual difference between 'class' and 'struct' in C++ is that 'class' defaults to
private, so it doesn't seem particularly meaningful in CIR, since you're not modeling the access specifiers.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some extra background: the distinction currently tracks some missing CodeGen support (as done by @sitio-couto a year go), but I'm in favor removing in favor of only one (to distinguish from unions) in the future if it's not used by anything else.