-
Notifications
You must be signed in to change notification settings - Fork 99
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
[CIR][Lowering] Add the concept of simple lowering and use it for unary fp2fp operations #806
Conversation
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.
LGTM with some minor nits
@@ -38,8 +38,13 @@ include "mlir/IR/CommonAttrConstraints.td" | |||
// CIR Ops | |||
//===----------------------------------------------------------------------===// | |||
|
|||
class LoweringInfo { |
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.
Add a comment here explaining that this is used to communicate to tablegen some direct lowering approach and perhaps an example of how it can be used.
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.
Updated.
@@ -38,8 +38,13 @@ include "mlir/IR/CommonAttrConstraints.td" | |||
// CIR Ops | |||
//===----------------------------------------------------------------------===// | |||
|
|||
class LoweringInfo { | |||
bit simpleLowering = 0; | |||
string llvmOpName; |
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.
How about we just have a single string simpleLoweringLLVMOpName = ""
variable, and if it's defined to anything other than empty it implies simple lowering?
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.
Yeah I'm thinking about exactly the same thing... Updated.
…ment FP intrincis
1d4987a
to
c3afb00
Compare
…ry fp2fp operations (llvm#806) This PR is the continuation and refinement of PR llvm#434 which is originally authored by @philnik777 . Does not update it in-place since I don't have commit access to Nikolas' repo. This PR basically just rebases llvm#434 onto the latest `main`. I also updated some naming used in the original PR to keep naming styles consistent. Co-authored-by: Nikolas Klauser <nikolasklauser@berlin.de>
…ry fp2fp operations (llvm#806) This PR is the continuation and refinement of PR llvm#434 which is originally authored by @philnik777 . Does not update it in-place since I don't have commit access to Nikolas' repo. This PR basically just rebases llvm#434 onto the latest `main`. I also updated some naming used in the original PR to keep naming styles consistent. Co-authored-by: Nikolas Klauser <nikolasklauser@berlin.de>
…ry fp2fp operations (llvm#806) This PR is the continuation and refinement of PR llvm#434 which is originally authored by @philnik777 . Does not update it in-place since I don't have commit access to Nikolas' repo. This PR basically just rebases llvm#434 onto the latest `main`. I also updated some naming used in the original PR to keep naming styles consistent. Co-authored-by: Nikolas Klauser <nikolasklauser@berlin.de>
…ry fp2fp operations (llvm#806) This PR is the continuation and refinement of PR llvm#434 which is originally authored by @philnik777 . Does not update it in-place since I don't have commit access to Nikolas' repo. This PR basically just rebases llvm#434 onto the latest `main`. I also updated some naming used in the original PR to keep naming styles consistent. Co-authored-by: Nikolas Klauser <nikolasklauser@berlin.de>
This PR is the continuation and refinement of PR #434 which is originally authored by @philnik777 . Does not update it in-place since I don't have commit access to Nikolas' repo.
This PR basically just rebases #434 onto the latest
main
. I also updated some naming used in the original PR to keep naming styles consistent.