User Details
- User Since
- Apr 17 2020, 11:03 AM (39 w, 6 d)
Tue, Jan 12
Wed, Jan 6
Make error message concise.
Better error messages
Simplify by moving check in parseFunctionLikeOp
This is in preparation of the visibility related changes discussed in https://llvm.discourse.group/t/external-function-declaration-has-changed-help-needed/2357. This change disallows attributes that sym_visibility/sym_name/type etc which are parsed/inferred from other places to be present in the attribute dictionary. This way, for the follow on changes, the function parser has just one parsed visibility value (parsed as keyword private/public/nested) to consider instead of trying to also look into the parsed attribute dictionary.
Tue, Dec 29
Mon, Dec 28
Use if-else
Dec 10 2020
Dec 9 2020
Address feedback
Dec 8 2020
Remove test case
Dec 4 2020
Dec 3 2020
Address feedback
Fix clang-tidy warning
Dec 2 2020
River's feedback
Not really. Note that both
Dec 1 2020
The fix here is such that TableGen generated inferReturnTypes takes precedence over the one generated by the InferTypeOpInterface. Just tagging the Op with the trait is not sufficient since I assume we also want the generated inferReturnTypes. The other possible fix is to avoid generating the TableGen generated inferReturnTypes method when the Op is tagged with DeclareOpInterfaceMethods<InferTypeOpInterface> and delegate the definition of the method to the user. I've erred on the side of using the TableGen one.
Nov 24 2020
Ping.
Nov 20 2020
Nov 17 2020
Nov 16 2020
Fix nit
Nov 13 2020
Attempt to resolve merge conflicts
Update comment.
Nov 12 2020
AFAICT it's not possible today to hit this, so this turns out to be just defensive programming. MemRef element type is either int/float/index/complex or a vector (which itself can only have int or float elements) and all these types seems to be convertible to LLVM types without any failures. As such, I don't think we will run into this issue today, but will guard against future extensions to MemRef types.
Remove leftover check.
This addresses the nit in https://reviews.llvm.org/D90803 more generically.
Nov 9 2020
This is in prep for the verification check I'll be adding to disallow public declarations. Several place (in and out-of-tree) now need to mark declarations private and its much short to use of these functions.
I already push this in, but we can revisit this if there is a better way. Could you give an example where the confusion arises? To me, the last instruction seems more relevant because that's where something is missing. Are you suggesting doing:
Use 0xdeadbeef for allocated pointer
Address feedback
Nov 8 2020
Use llvm::is_contained() in parseOptionalKeyword
Nov 6 2020
Review feedback
I did think of moving the visibility enums to ODS, but I wasn't sure how operations like ModuleOp and FuncOp that are defined outside ODS will use it. Also StrEnumAttr still prints a quoted string I think, so beyond that, we would also need some kind of qualifier in the declarative assembly syntax to indicate that the custom parser needs to print it without quotes (something like noquotes($sym_visibility)). So did not go down that path.
Update syntax to not have quotes around visibility
I don't think there's a way to do that with the declarative parses though, and we use that std.global_memref so I avoided diverging the syntax between the two. But I do agree that quotes are not that readable. I can change this to be unquoted, but then we would need to change global_memref to use the similar syntax, likely by using the custom directive. @rriddle/@mehdi_amin, what do you think? It almost seems that the SymbolTable can provide helper functions to print/parse visibility that can be reused here across ops.
Nov 5 2020
@mehdi_amini or @ftynse, can one you take a look as well?