-
Notifications
You must be signed in to change notification settings - Fork 14.5k
[mlir][LLVM] Add support for constant struct with multiple fields #102752
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
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 | ||||
---|---|---|---|---|---|---|
|
@@ -16,6 +16,7 @@ | |||||
#include "mlir/Dialect/LLVMIR/LLVMAttrs.h" | ||||||
#include "mlir/Dialect/LLVMIR/LLVMInterfaces.h" | ||||||
#include "mlir/Dialect/LLVMIR/LLVMTypes.h" | ||||||
#include "mlir/IR/Attributes.h" | ||||||
#include "mlir/IR/Builders.h" | ||||||
#include "mlir/IR/BuiltinOps.h" | ||||||
#include "mlir/IR/BuiltinTypes.h" | ||||||
|
@@ -2710,32 +2711,38 @@ LogicalResult LLVM::ConstantOp::verify() { | |||||
} | ||||||
return success(); | ||||||
} | ||||||
if (auto structType = llvm::dyn_cast<LLVMStructType>(getType())) { | ||||||
if (structType.getBody().size() != 2 || | ||||||
structType.getBody()[0] != structType.getBody()[1]) { | ||||||
return emitError() << "expected struct type with two elements of the " | ||||||
"same type, the type of a complex constant"; | ||||||
if (auto structType = dyn_cast<LLVMStructType>(getType())) { | ||||||
auto arrayAttr = dyn_cast<ArrayAttr>(getValue()); | ||||||
if (!arrayAttr) { | ||||||
return emitOpError() << "expected array attribute for a struct constant"; | ||||||
} | ||||||
|
||||||
auto arrayAttr = llvm::dyn_cast<ArrayAttr>(getValue()); | ||||||
if (!arrayAttr || arrayAttr.size() != 2) { | ||||||
return emitOpError() << "expected array attribute with two elements, " | ||||||
"representing a complex constant"; | ||||||
ArrayRef<Type> elementTypes = structType.getBody(); | ||||||
if (arrayAttr.size() != elementTypes.size()) { | ||||||
return emitOpError() << "expected array attribute of size " | ||||||
<< elementTypes.size(); | ||||||
} | ||||||
auto re = llvm::dyn_cast<TypedAttr>(arrayAttr[0]); | ||||||
auto im = llvm::dyn_cast<TypedAttr>(arrayAttr[1]); | ||||||
if (!re || !im || re.getType() != im.getType()) { | ||||||
return emitOpError() | ||||||
<< "expected array attribute with two elements of the same type"; | ||||||
for (auto elementTy : elementTypes) { | ||||||
if (!isa<IntegerType, FloatType, LLVMPPCFP128Type>(elementTy)) { | ||||||
return emitOpError() << "expected struct element types to be floating " | ||||||
"point type or integer type"; | ||||||
} | ||||||
} | ||||||
|
||||||
Type elementType = structType.getBody()[0]; | ||||||
if (!llvm::isa<IntegerType, Float16Type, Float32Type, Float64Type>( | ||||||
elementType)) { | ||||||
return emitError() | ||||||
<< "expected struct element types to be floating point type or " | ||||||
"integer type"; | ||||||
for (size_t i = 0; i < elementTypes.size(); ++i) { | ||||||
Attribute element = arrayAttr[i]; | ||||||
if (!isa<IntegerAttr, FloatAttr>(element)) { | ||||||
return emitOpError() | ||||||
<< "expected struct element attribute types to be floating " | ||||||
"point type or integer type"; | ||||||
} | ||||||
auto elementType = cast<TypedAttr>(element).getType(); | ||||||
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.
Suggested change
Didn't see this before. I think we should dyn_cast here (as it was before). In case the attribute does not implement the TypedAttr interface we want to return an error and not assert. 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. Is this cast fallible? We have already checked that the attribute is either an 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. True you are right! Then this PR is ready to go. Do you have commit rights? 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.
No. 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 will land it then. |
||||||
if (elementType != elementTypes[i]) { | ||||||
return emitOpError() | ||||||
<< "struct element at index " << i << " is of wrong type"; | ||||||
} | ||||||
} | ||||||
|
||||||
return success(); | ||||||
} | ||||||
if (auto targetExtType = dyn_cast<LLVMTargetExtType>(getType())) { | ||||||
|
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.
Shouldn't this check now be recursive, given that one can define nested struct constants?