-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[CIR] Upstream DivOp for ComplexType #153796
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
Changes from 4 commits
6613d4c
6879b70
fc62a60
511115f
6745410
a9a9fca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2966,7 +2966,7 @@ def CIR_ComplexSubOp : CIR_Op<"complex.sub", [ | |||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| //===----------------------------------------------------------------------===// | ||||||||||||||
| // ComplexMulOp | ||||||||||||||
| // ComplexMulOp & ComplexDivOp | ||||||||||||||
| //===----------------------------------------------------------------------===// | ||||||||||||||
|
|
||||||||||||||
| def CIR_ComplexRangeKind : CIR_I32EnumAttr< | ||||||||||||||
|
|
@@ -2985,8 +2985,8 @@ def CIR_ComplexMulOp : CIR_Op<"complex.mul", [ | |||||||||||||
| The `cir.complex.mul` operation takes two complex numbers and returns | ||||||||||||||
| their product. | ||||||||||||||
|
|
||||||||||||||
| Range is used to select the implementation used when the operation | ||||||||||||||
| is lowered to the LLVM dialect. For multiplication, 'improved', | ||||||||||||||
| The `range` attribute is used to select the algorithm used when the | ||||||||||||||
| operation is lowered to the LLVM dialect. For multiplication, 'improved', | ||||||||||||||
| 'promoted', and 'basic' are all handled equivalently, producing the | ||||||||||||||
| algebraic formula with no special handling for NaN value. If 'full' is | ||||||||||||||
| used, a runtime-library function is called if one of the intermediate | ||||||||||||||
|
|
@@ -3013,6 +3013,47 @@ def CIR_ComplexMulOp : CIR_Op<"complex.mul", [ | |||||||||||||
| }]; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| def CIR_ComplexDivOp : CIR_Op<"complex.div", [ | ||||||||||||||
| Pure, SameOperandsAndResultType | ||||||||||||||
| ]> { | ||||||||||||||
| let summary = "Complex division"; | ||||||||||||||
| let description = [{ | ||||||||||||||
| The `cir.complex.div` operation takes two complex numbers and returns | ||||||||||||||
| their quotient. | ||||||||||||||
|
|
||||||||||||||
| The `range` attribute is used to select the algorithm used when | ||||||||||||||
| the operation is lowered to the LLVM dialect. For division, 'improved' | ||||||||||||||
| producing the Smith's algorithms for Complex division with no special | ||||||||||||||
|
||||||||||||||
| producing the Smith's algorithms for Complex division with no special | |
| produces Smith's algorithms for Complex division with no additional |
Smith's algorithm is a way of avoiding most intermediate NaN values,so 'additional' is better here.
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.
Thank you so much for the suggestions and improvements. I applied them and aligned MulOp
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.
Please add a statment mentioning that the range attribute is ignored for complex integer types.
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.
I can add one, but I think we already mentioned that it's for FP. Does it still need that note?
For complex types with floating-point components, the
rangeattribute specifies the algorithm to be used when
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.
I added a note at the end about that
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.
I think it's helpful to be explicit about what happens. The FP note says what range means for FP types, but it doesn't explicitly say what happens for other types.
Outdated
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.
| using the algebraic formula. We only fall back on Smith's algorithm when | |
| the target does not support a higher precision type. Also, this only | |
| applies to floating-point types with no special handling for NaN values. | |
| using the algebraic formula, with no additional handling for NaN values. | |
| We fall back on Smith's algorithm when the target does not support a | |
| higher precision type. |
The comment about floating-point types applies to this entire paragraph. For integer types, the range attribute is ignored and we always lower to the algebraic formula. It's probably best to say that in a separate paragraph.
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.