stuij added a comment. In D78190#2018685 <https://reviews.llvm.org/D78190#2018685>, @fpetrogalli wrote:
> 1. Shouldn't we test also that the parser is happy with the following > expressions? Added. Thanks. > 2. Would it make sense to to split this patch into 2 separate patches? One > that defines the enums and interfaces for `bfloat`, and one that does the > actual parsing/emission in the IR? I suspect there is much intertwine going > on, so probably not - in that case, I am happy for everything to go via a > single patch. Yes, the parts are a bit intertwined. There are some lines along which we can cut the patch, but I personally don't feel we'd gain enough clarity to justify the cost. It's not a very sexy patch in my opinion, just a number of places to add the new type to. Also, this is also a nice unit of functionality to be able to test against. And lastly, I think we already had a good number of eyes on this patch. It seems like a duplication of effort to have people again review the different parts. > 3. Do you need those changes in the Hexagon and x86 backend? Could they be > submitted separately, with some testing? This is a backend agnostic patch. If the Hexagon and/or x86 communities want to make use of the IR type in some way, then yes, they can for sure submit the necessary patches. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78190/new/ https://reviews.llvm.org/D78190 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
