-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[flang-rt] Fix TypeCategory for quad-precision COMPLEX #168090
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
Conversation
Modify the TypeCategory for quad-precision COMPLEX to CFI_type_float128_Complex so it matches the TypeCode returned by SELECT TYPE lowering. Fixes llvm#134565
|
I'd like to add a testcase (check the generated LLVM IR for the correct type selection). Where's the best place to put it? |
You could add a test like this using a |
My question was more in the lines of where in the directory structure is more adequate. Although the fix is in flang-rt, this is technically lowering. Maybe |
Ah sorry, so you want to test SELECT TYPE lowering? If so, I think you should make a small FIR test using fir.select_type with complex and check that you get the code you expect. See https://github.com/llvm/llvm-project/blob/main/flang/test/Lower/select-type-2.fir as an example (except these kinds of test should live in flang/test/Fir). You can generate your input from Fortran with bbc -emit-fir, but try to keep the input very small, focused on the testing the resulting code. |
Actually, scratch that. The resulting IR will be the same with or without the fix (it will branch differently if the type is 16 or 17). I added a runtime check as you mentioned earlier. If you think it's OK, I'll merge this. |
🐧 Linux x64 Test Results
|
jeanPerier
left a comment
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
Modify the TypeCategory for quad-precision COMPLEX to CFI_type_float128_Complex so it matches the TypeCode returned by SELECT TYPE lowering.
Fixes #134565