Page MenuHomePhabricator

jurahul (Rahul Joshi)
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 17 2020, 11:03 AM (15 w, 6 d)

Recent Activity

Today

jurahul added a comment to D85060: [MLIR][TableGen] Fix ambiguous build methods when inferring result types..

I agree. A more general solution might be to generate all "collective" variants first, and then have a way for the build methods that will be ambiguous with any of them automatically filtered out, so that we don't have to guard their generation, just their emission. That might need the OpMethodSignature to have more structure than the current string members. I will file a bug for this assuming folks agree.

Why not have opClass.newMethod flag this? If adding a new method to the class with the same signature, flag it (return nullptr or some such, depending on where the collision occurs it is either fine or an error). Start with the build methods users requested first and then add autogenerated ones as long as they don't conflict. Then we don't have to do gymnastics here. I'd say even the non-collective ones are the more user friendly ones. Although in cases of conflict it provides the same signature so perhaps it doesn't matter which one gets generated.

Fri, Aug 7, 9:35 AM · Restricted Project

Yesterday

jurahul added inline comments to D85382: [MLIR] Add getSizeInBits() for tensor of complex.
Thu, Aug 6, 2:27 PM · Restricted Project
jurahul added inline comments to D85449: [mlir][SCF] Add utility to outline the then and else branches of an scf.IfOp.
Thu, Aug 6, 10:48 AM · Restricted Project
jurahul added inline comments to D85073: [MLIR] Make gpu.launch_func rewrite pattern part of the LLVM lowering pass..
Thu, Aug 6, 8:26 AM · Restricted Project

Tue, Aug 4

jurahul updated the diff for D84897: [MLIR] Add support for defining and using Op specific analysis.

Address clang-tidy warnings

Tue, Aug 4, 2:14 PM · Restricted Project
jurahul updated the diff for D84897: [MLIR] Add support for defining and using Op specific analysis.

Rebase

Tue, Aug 4, 1:21 PM · Restricted Project
jurahul added a comment to D85060: [MLIR][TableGen] Fix ambiguous build methods when inferring result types..

Thanks. @jpienaar, waiting to give you an opportunity to look as well.

Tue, Aug 4, 1:19 PM · Restricted Project
jurahul committed rG1d6a724aa1c1: [MLIR] Change FunctionType::get() and TupleType::get() to use TypeRange (authored by jurahul).
[MLIR] Change FunctionType::get() and TupleType::get() to use TypeRange
Tue, Aug 4, 12:44 PM
jurahul closed D85075: [MLIR] Change FunctionType::get() and TupleType::get() to use TypeRange.
Tue, Aug 4, 12:44 PM · Restricted Project
jurahul updated the diff for D85075: [MLIR] Change FunctionType::get() and TupleType::get() to use TypeRange.

Fix a clang-format warning

Tue, Aug 4, 11:46 AM · Restricted Project
jurahul updated the diff for D85075: [MLIR] Change FunctionType::get() and TupleType::get() to use TypeRange.

Address nits

Tue, Aug 4, 11:09 AM · Restricted Project

Mon, Aug 3

jurahul updated the diff for D85075: [MLIR] Change FunctionType::get() and TupleType::get() to use TypeRange.

Remove OperationSupport.h include

Mon, Aug 3, 6:30 PM · Restricted Project
jurahul added a comment to D85075: [MLIR] Change FunctionType::get() and TupleType::get() to use TypeRange.

I did run clang-format, giving it another go.

Mon, Aug 3, 6:02 PM · Restricted Project
jurahul updated the diff for D85075: [MLIR] Change FunctionType::get() and TupleType::get() to use TypeRange.

Review feedback

Mon, Aug 3, 6:02 PM · Restricted Project
jurahul added a comment to D85060: [MLIR][TableGen] Fix ambiguous build methods when inferring result types..

I agree. A more general solution might be to generate all "collective" variants first, and then have a way for the build methods that will be ambiguous with any of them automatically filtered out, so that we don't have to guard their generation, just their emission. That might need the OpMethodSignature to have more structure than the current string members. I will file a bug for this assuming folks agree.

Mon, Aug 3, 11:31 AM · Restricted Project
jurahul updated the diff for D85075: [MLIR] Change FunctionType::get() and TupleType::get() to use TypeRange.

Remove commented out code.

Mon, Aug 3, 8:51 AM · Restricted Project

Sun, Aug 2

jurahul updated the diff for D85075: [MLIR] Change FunctionType::get() and TupleType::get() to use TypeRange.

Fix flang build failure, address some clang-tidy issues

Sun, Aug 2, 5:58 PM · Restricted Project

Sat, Aug 1

jurahul requested review of D85075: [MLIR] Change FunctionType::get() and TupleType::get() to use TypeRange.
Sat, Aug 1, 9:47 AM · Restricted Project

Fri, Jul 31

jurahul edited reviewers for D85060: [MLIR][TableGen] Fix ambiguous build methods when inferring result types., added: jpienaar, stephenneuendorffer; removed: antiagainst.
Fri, Jul 31, 5:04 PM · Restricted Project
jurahul requested review of D85060: [MLIR][TableGen] Fix ambiguous build methods when inferring result types..
Fri, Jul 31, 5:02 PM · Restricted Project
jurahul committed rGeb8c72cb0d89: [MLIR][NFC] Add FuncOp::getArgumentTypes() (authored by jurahul).
[MLIR][NFC] Add FuncOp::getArgumentTypes()
Fri, Jul 31, 1:18 PM
jurahul closed D85038: [MLIR][NFC] Add FuncOp::addArgument() and FuncOp::getArgumentTypes().
Fri, Jul 31, 1:18 PM · Restricted Project
jurahul updated the diff for D85038: [MLIR][NFC] Add FuncOp::addArgument() and FuncOp::getArgumentTypes().

Remove FuncOp::getArgument()

Fri, Jul 31, 11:40 AM · Restricted Project
jurahul added inline comments to D85038: [MLIR][NFC] Add FuncOp::addArgument() and FuncOp::getArgumentTypes().
Fri, Jul 31, 11:38 AM · Restricted Project
jurahul requested review of D85038: [MLIR][NFC] Add FuncOp::addArgument() and FuncOp::getArgumentTypes().
Fri, Jul 31, 10:53 AM · Restricted Project

Thu, Jul 30

jurahul committed rGa34a8d52605a: [MLIR][NFC] Add SymbolUse::UseRange::empty() (authored by jurahul).
[MLIR][NFC] Add SymbolUse::UseRange::empty()
Thu, Jul 30, 3:19 PM
jurahul closed D84984: [MLIR][NFC] Add SymbolUse::UseRange::empty().
Thu, Jul 30, 3:19 PM · Restricted Project
jurahul requested review of D84984: [MLIR][NFC] Add SymbolUse::UseRange::empty().
Thu, Jul 30, 2:18 PM · Restricted Project

Wed, Jul 29

jurahul added a reviewer for D84897: [MLIR] Add support for defining and using Op specific analysis: mehdi_amini.
Wed, Jul 29, 5:23 PM · Restricted Project
jurahul requested review of D84897: [MLIR] Add support for defining and using Op specific analysis.
Wed, Jul 29, 5:22 PM · Restricted Project

Tue, Jul 28

jurahul committed rG706d992cedaf: [NFC] Add getArgumentTypes() to Region (authored by jurahul).
[NFC] Add getArgumentTypes() to Region
Tue, Jul 28, 6:28 PM
jurahul closed D84807: [NFC] Add getArgumentTypes() to Region.
Tue, Jul 28, 6:28 PM · Restricted Project
jurahul committed rG0b161def6cac: [MLIR] Add unit test for tblgen Op build methods (authored by jurahul).
[MLIR] Add unit test for tblgen Op build methods
Tue, Jul 28, 3:44 PM
jurahul closed D84074: [MLIR] Add unit test for tblgen Op build methods.
Tue, Jul 28, 3:44 PM · Restricted Project
jurahul requested review of D84807: [NFC] Add getArgumentTypes() to Region.
Tue, Jul 28, 3:26 PM · Restricted Project
jurahul added a comment to D84074: [MLIR] Add unit test for tblgen Op build methods.

I have added a comment explaining why I am testing these 3 combinations in particulat:

I think you have a reason for this configuration, but it isn't clear just looking at the code why (for example) multiple variadic args need not be tested.

Tue, Jul 28, 10:32 AM · Restricted Project
jurahul updated the diff for D84074: [MLIR] Add unit test for tblgen Op build methods.

Address review comment

Tue, Jul 28, 10:30 AM · Restricted Project

Wed, Jul 22

jurahul committed rGed88cd77d4a0: [NFC] Simplify `splitLiteralAndReplacement` function (authored by jurahul).
[NFC] Simplify `splitLiteralAndReplacement` function
Wed, Jul 22, 3:33 PM
jurahul closed D84178: [NFC] Simplify `splitLiteralAndReplacement` function.
Wed, Jul 22, 3:33 PM · Restricted Project
jurahul committed rGe6ea5b388b0d: [LLVM] Update formatv() documentation to clarify no escape for `}` (authored by jurahul).
[LLVM] Update formatv() documentation to clarify no escape for `}`
Wed, Jul 22, 3:31 PM
jurahul closed D83888: [LLVM] Update formatv() documentation to clarify no escape for `}`.
Wed, Jul 22, 3:31 PM · Restricted Project

Tue, Jul 21

jurahul added a comment to D84075: [MLIR] Add convenience specializations of OpBuilder create() and friends.

@ftynse, I read your document a bit, but another idea came to my mind which seems like an incremental change. Instead of the per-dialect builder you had suggested:

Tue, Jul 21, 7:43 AM · Restricted Project

Mon, Jul 20

jurahul added a comment to D84075: [MLIR] Add convenience specializations of OpBuilder create() and friends.

Agreed, in certain instances if clarity is desired, an explicit TypeRange{} can be still be added. But in many cases, things may be clear enough that the extra 'TypeRange{}' or ValueRange{} may be just noise, which is what I am attempting to address here. We have some instances within MLIR, but I suspect several out-of-tree instances that would read easier without these casts.

Mon, Jul 20, 9:39 AM · Restricted Project
jurahul added a reviewer for D84178: [NFC] Simplify `splitLiteralAndReplacement` function: sammccall.
Mon, Jul 20, 8:29 AM · Restricted Project
Herald added a project to D84178: [NFC] Simplify `splitLiteralAndReplacement` function: Restricted Project.
Mon, Jul 20, 8:29 AM · Restricted Project
jurahul updated the diff for D84075: [MLIR] Add convenience specializations of OpBuilder create() and friends.

Cleanup leftover macro code

Mon, Jul 20, 8:23 AM · Restricted Project
jurahul added a comment to D84075: [MLIR] Add convenience specializations of OpBuilder create() and friends.

This is my attempt to remove the use of Macros. With this, we essentially need one specialization class for each function we want to specialize. I tried to go down a different route as well, creating an instance of an (empty) specialization object with a templated () operator. The hope was that we can define this class just once, and say the builder has an instance of this class called 'create' and we could do something like

Mon, Jul 20, 8:21 AM · Restricted Project
jurahul updated the diff for D84075: [MLIR] Add convenience specializations of OpBuilder create() and friends.

Eliminate macros

Mon, Jul 20, 8:08 AM · Restricted Project
jurahul added inline comments to D83888: [LLVM] Update formatv() documentation to clarify no escape for `}`.
Mon, Jul 20, 8:06 AM · Restricted Project
jurahul updated the summary of D83888: [LLVM] Update formatv() documentation to clarify no escape for `}`.
Mon, Jul 20, 8:05 AM · Restricted Project
jurahul updated the diff for D83888: [LLVM] Update formatv() documentation to clarify no escape for `}`.

Review feedback

Mon, Jul 20, 8:04 AM · Restricted Project

Fri, Jul 17

jurahul added inline comments to D84075: [MLIR] Add convenience specializations of OpBuilder create() and friends.
Fri, Jul 17, 5:38 PM · Restricted Project
jurahul added inline comments to D84075: [MLIR] Add convenience specializations of OpBuilder create() and friends.
Fri, Jul 17, 4:45 PM · Restricted Project
jurahul updated the diff for D84074: [MLIR] Add unit test for tblgen Op build methods.

Fix clamg-tidy warnings

Fri, Jul 17, 4:19 PM · Restricted Project
jurahul added inline comments to D84075: [MLIR] Add convenience specializations of OpBuilder create() and friends.
Fri, Jul 17, 4:04 PM · Restricted Project
jurahul added a comment to D84075: [MLIR] Add convenience specializations of OpBuilder create() and friends.

Note, there can be more specializations that can be added, but I wanted to get this out to get some feedback. Some next items to handle are (1) canonicalize various CallOp build parameters (some have function followed by result types, and other result types followed by functions) and then add a specialization for that, and (2) one for branch instructions. The goal is to make the calls to create/replace etc slightly simpler and eliminate/reduce the need to type ArrayRef<Type> or ArrayRef<Value> in front of a collection of types or values passed into these functions.

Fri, Jul 17, 3:59 PM · Restricted Project
Herald added a project to D84075: [MLIR] Add convenience specializations of OpBuilder create() and friends: Restricted Project.
Fri, Jul 17, 3:54 PM · Restricted Project
jurahul added reviewers for D84074: [MLIR] Add unit test for tblgen Op build methods: rriddle, jpienaar.
Fri, Jul 17, 3:46 PM · Restricted Project
jurahul abandoned D84073: [MLIR] Add unit test for tblgen Op build methods.
Fri, Jul 17, 3:45 PM · Restricted Project
Herald added a project to D84074: [MLIR] Add unit test for tblgen Op build methods: Restricted Project.
Fri, Jul 17, 3:45 PM · Restricted Project
Herald added a project to D84073: [MLIR] Add unit test for tblgen Op build methods: Restricted Project.
Fri, Jul 17, 3:41 PM · Restricted Project

Thu, Jul 16

jurahul committed rG3c6a518a2fd2: [NFC] Use appropriate names for `for_each` and `transform` template parameters (authored by jurahul).
[NFC] Use appropriate names for `for_each` and `transform` template parameters
Thu, Jul 16, 9:35 AM
jurahul closed D83848: [NFC] Use appropriate names for `for_each` and `transform` template parameters.
Thu, Jul 16, 9:35 AM · Restricted Project
jurahul committed rG764931d248d5: [MLIR][TableGen] Add default value for named attributes for 2 more build methods (authored by jurahul).
[MLIR][TableGen] Add default value for named attributes for 2 more build methods
Thu, Jul 16, 9:33 AM
jurahul closed D83839: [MLIR][TableGen] Add default value for named attributes for 2 more build methods.
Thu, Jul 16, 9:33 AM · Restricted Project
jurahul added inline comments to D83839: [MLIR][TableGen] Add default value for named attributes for 2 more build methods.
Thu, Jul 16, 9:24 AM · Restricted Project
jurahul committed rG2e046be90e51: [flang] Adopt NoRegionArguments (WhereOp) and ParentOneOf (ResultOp) traits (authored by jurahul).
[flang] Adopt NoRegionArguments (WhereOp) and ParentOneOf (ResultOp) traits
Thu, Jul 16, 8:35 AM
jurahul closed D83522: [flang] Adopt NoRegionArguments (WhereOp) and ParentOneOf (ResultOp) traits.
Thu, Jul 16, 8:35 AM · Restricted Project
jurahul committed rG86ae0dd7f754: [MLIR] Add OpPrintingFlags to IRPrinterConfig. (authored by jurahul).
[MLIR] Add OpPrintingFlags to IRPrinterConfig.
Thu, Jul 16, 8:06 AM
jurahul closed D83930: [MLIR] Add OpPrintingFlags to IRPrinterConfig..
Thu, Jul 16, 8:06 AM · Restricted Project
jurahul updated the diff for D83522: [flang] Adopt NoRegionArguments (WhereOp) and ParentOneOf (ResultOp) traits.

Fix build failure

Thu, Jul 16, 7:36 AM · Restricted Project

Wed, Jul 15

jurahul added a reviewer for D83930: [MLIR] Add OpPrintingFlags to IRPrinterConfig.: mehdi_amini.
Wed, Jul 15, 10:27 PM · Restricted Project
Herald added a project to D83930: [MLIR] Add OpPrintingFlags to IRPrinterConfig.: Restricted Project.
Wed, Jul 15, 10:27 PM · Restricted Project
jurahul added a comment to D83522: [flang] Adopt NoRegionArguments (WhereOp) and ParentOneOf (ResultOp) traits.

Thanks. I have created a pull request as well here: https://github.com/flang-compiler/f18-llvm-project/pull/275.

Wed, Jul 15, 10:25 PM · Restricted Project
jurahul added a comment to D83522: [flang] Adopt NoRegionArguments (WhereOp) and ParentOneOf (ResultOp) traits.

Holding off on pushing till the dev branch issue is clarified. Please let me know if Mehdi'd understanding is correct. How do you plan to manage API breaking changes in MLIR that may not be in this dev branch?

Wed, Jul 15, 6:24 PM · Restricted Project
jurahul committed rGa3ad8f92b44d: [MLIR] Add type checking capability to RegionBranchOpInterface (authored by jurahul).
[MLIR] Add type checking capability to RegionBranchOpInterface
Wed, Jul 15, 11:14 AM
jurahul closed D82829: [MLIR] Add type checking capability to RegionBranchOpInterface.
Wed, Jul 15, 11:14 AM · Restricted Project
jurahul updated the summary of D83888: [LLVM] Update formatv() documentation to clarify no escape for `}`.
Wed, Jul 15, 10:32 AM · Restricted Project
jurahul updated the summary of D83888: [LLVM] Update formatv() documentation to clarify no escape for `}`.
Wed, Jul 15, 10:32 AM · Restricted Project
jurahul added reviewers for D83888: [LLVM] Update formatv() documentation to clarify no escape for `}`: sammccall, zturner, mehdi_amini.
Wed, Jul 15, 10:28 AM · Restricted Project
Herald added a project to D83888: [LLVM] Update formatv() documentation to clarify no escape for `}`: Restricted Project.
Wed, Jul 15, 10:26 AM · Restricted Project
jurahul abandoned D83837: [LLVM] Fix handling of escaped closing brace `}` in `formatv` (under an option).

Abandoning this one, will start a new one for the documentation changes and some simplification to the code as well.

Wed, Jul 15, 9:54 AM · Restricted Project
jurahul updated the diff for D82829: [MLIR] Add type checking capability to RegionBranchOpInterface.

Also undo changes to IfOp::getSuccessorRegions() since we are now passing an array of null attributes during verification.

Wed, Jul 15, 9:05 AM · Restricted Project
jurahul updated the diff for D82829: [MLIR] Add type checking capability to RegionBranchOpInterface.

Adopt Region::getNumArguments()

Wed, Jul 15, 9:02 AM · Restricted Project
jurahul added a comment to D83837: [LLVM] Fix handling of escaped closing brace `}` in `formatv` (under an option).

That's a reasonable course of action as well. I could look into fixing all the in-tree uses of formatv() to conform to the strict mode, but I do have concern about out-of-tree uses breaking with this and leading to unnecessary API churn, so for that purpose we would potentially need to keep the legacy mode. So overall, I think updating the documentation seems reasonable. I'll update the review for that.

Wed, Jul 15, 8:48 AM · Restricted Project

Tue, Jul 14

jurahul added reviewers for D83848: [NFC] Use appropriate names for `for_each` and `transform` template parameters: rriddle, mehdi_amini.
Tue, Jul 14, 10:35 PM · Restricted Project
Herald added a project to D83848: [NFC] Use appropriate names for `for_each` and `transform` template parameters: Restricted Project.
Tue, Jul 14, 10:34 PM · Restricted Project
jurahul updated the diff for D82829: [MLIR] Add type checking capability to RegionBranchOpInterface.

Address River's comments.

Tue, Jul 14, 7:04 PM · Restricted Project
jurahul added inline comments to D82829: [MLIR] Add type checking capability to RegionBranchOpInterface.
Tue, Jul 14, 6:38 PM · Restricted Project
jurahul added a comment to D83522: [flang] Adopt NoRegionArguments (WhereOp) and ParentOneOf (ResultOp) traits.

Ping. Not sure if flang is accepting such changes, if not please let me know and I will abandon this.

Tue, Jul 14, 6:31 PM · Restricted Project
jurahul added inline comments to D83839: [MLIR][TableGen] Add default value for named attributes for 2 more build methods.
Tue, Jul 14, 6:30 PM · Restricted Project
jurahul added reviewers for D83839: [MLIR][TableGen] Add default value for named attributes for 2 more build methods: jpienaar, rriddle, mehdi_amini.
Tue, Jul 14, 6:28 PM · Restricted Project
Herald added a project to D83839: [MLIR][TableGen] Add default value for named attributes for 2 more build methods: Restricted Project.
Tue, Jul 14, 6:28 PM · Restricted Project
jurahul added a reviewer for D83837: [LLVM] Fix handling of escaped closing brace `}` in `formatv` (under an option): zturner.
Tue, Jul 14, 6:26 PM · Restricted Project
jurahul added a comment to D83837: [LLVM] Fix handling of escaped closing brace `}` in `formatv` (under an option).

I ran into this issue when trying to attach default values to a parameter in the code generated by TableGen, which uses formatv(). Adding just {} does not work, so I thought {{}} should work based on the comments, but the code does not handle this. When testing, I found that there is a lot of existing code and tests that rely on this current (legacy) behavior, so decided to only enable the new mode optionally. Ideally, we should get rid of the legacy behavior, but it seems there are too many tests to fix (in clang, clangd etc), so I am not handling that with this change.

Tue, Jul 14, 6:09 PM · Restricted Project
jurahul added reviewers for D83837: [LLVM] Fix handling of escaped closing brace `}` in `formatv` (under an option): bkramer, mehdi_amini, sammccall.
Tue, Jul 14, 6:06 PM · Restricted Project
Herald added a project to D83837: [LLVM] Fix handling of escaped closing brace `}` in `formatv` (under an option): Restricted Project.
Tue, Jul 14, 6:03 PM · Restricted Project
jurahul committed rGe2b716105be3: [MLIR] Add argument related API to Region (authored by jurahul).
[MLIR] Add argument related API to Region
Tue, Jul 14, 9:29 AM
jurahul closed D83599: [MLIR] Add argument related API to Region.
Tue, Jul 14, 9:28 AM · Restricted Project
jurahul committed rG256d44811eab: [MLIR] [TableGen] Avoid generating an assert which is always true. (authored by jurahul).
[MLIR] [TableGen] Avoid generating an assert which is always true.
Tue, Jul 14, 9:20 AM