Skip to content

[Moore] Add missing conversions to open array typesAdd support for OpenUnpackedArrayType constants - #11058

Open
ayush213 wants to merge 2 commits into
llvm:mainfrom
ayush213:my-new-feature
Open

[Moore] Add missing conversions to open array typesAdd support for OpenUnpackedArrayType constants#11058
ayush213 wants to merge 2 commits into
llvm:mainfrom
ayush213:my-new-feature

Conversation

@ayush213

Copy link
Copy Markdown

No description provided.

@ayush213

Copy link
Copy Markdown
Author

you checked ??

@TaoBi22 TaoBi22 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for looking at this @ayush213 - looks like you're still running into formatting issues on CI. You should be able to get around those by running clang-format on Expressions.cpp.

It also looks like you have two PR messages in your title - I'm guessing only the second is meant to be there? If you'd be able to add a little bit of information in the PR description in case anyone comes back to look at this, that would also be great!

Otherwise just a couple of comments below


llvm::SmallVector<mlir::Value> elemVals;
moore::ConstantOp constOp;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: unrelated change

}

// Create open array value
auto arrType = moore::UnpackedArrayType::get(getContext(), elementType, 0);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It looks like we already have an OpenUnpackedArrayType in Moore, I would think we probably want to fetch that type instead of just getting an UnpackedArrayType of size 0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants