Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[CLANG] Full support of complex multiplication and division. #81514
[CLANG] Full support of complex multiplication and division. #81514
Changes from all commits
13fd739
eb9a35c
d3c4f78
4aa0925
fcd5665
2ddba9a
e62c462
f635f94
1d61aa6
148b6ce
5aa2711
ba9a8da
9098908
52181c7
a9449de
e20741e
0d97b9b
bc3fa4f
ec6296f
3117dbd
0a08598
a558d31
dec045b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 didn't realize these had made it into the 18.0 release when I suggested that we could remove them. We would need at least one release where they are marked as deprecated, but since they are standard gcc options, maybe it makes sense to just keep them and have them alias to the new option as:
-fcx-limited-range --> -fcomplex-arithmetic=basic
-fcx-fortran-rules --> -fcomplex-arithmetic=improved
-fno-cx-limited-range --> -fcomplex-arithmetic=full
-fno-cx-fortran-rules --> -fcomplex-arithmetic=full
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.
The problem with aliasing is that the user would be allowed to write something like this:
-fcx-limited-range -fcomplex-arithmetic=improved
This will generate a warning like this:
warning: overriding '-fcomplex-arithmetic=basic' option with '-fcomplex-arithmetic=improved' [-Woverriding-option]
This warning is a bit mis-leading and doesn't reflect the option used in the command line. Not sure this can be corrected.
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.
Sorry. I meant "aliasing" in the non-technical sense of "having the same meaning." How that gets implemented is another matter. I think the driver could translate them to the same cc1 option.
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.
yes there is a way of doing that:
That still produces the misleading warning for: -fcx-limited-range -fcomplex-arithmetic=improved
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.
What I meant to suggest is that you can leave the driver-level options as if they were independent, but when we process them in RenderFloatingPointOptions, -fcx-limited-range and -fcomplex-arithmetic=basic (for example), would add the same cc1 option. Since the warning is generated from the RenderFloatingPointOptions we should be able to make that report the expected output.
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'd appreciate a comment here explaining why this is the correct check, so that future people reading this code can understand why this is being doing.