Support floor division as intrinsic function#2372
Conversation
So it can be used in intrinsic_function_registry
ubaidsk
force-pushed
the
intrinsic_floor_div
branch
2 times, most recently
from
b247454 to
4b917cf
Compare
ubaidsk
force-pushed
the
intrinsic_floor_div
branch
from
8e80b97 to
235117b
Compare
|
Ready. |
|
|
||
| ASR::expr_t* perform_casting(ASR::expr_t* expr, ASR::ttype_t* src, | ||
| ASR::ttype_t* dest, Allocator& al, | ||
| const Location& loc); |
There was a problem hiding this comment.
What is the idea behind these? I think we do not want to do any kind of automatic casting in ASR, nor be responsible for determining what proper casting must be done.
There was a problem hiding this comment.
I think we do not want to do any kind of automatic casting in ASR
It is actually not for automatic casting. The floordiv intrinsic we defined in this PR is generalized so that it can work on any type (from the supported types of int, unsigned int, real, logical). The casting function is used to cast the intermediate temporary variable to the appropriate return type.
There was a problem hiding this comment.
It is also used to cast the function arguments to the needed intermediate type.
There was a problem hiding this comment.
Is this a helper function to make make_Cast_t_value more convenient, but you still tell the initial and final type explicitly?
There was a problem hiding this comment.
Is this a helper function to make make_Cast_t_value more convenient, but you still tell the initial and final type explicitly?
Yes. The way it helps is we need not worry about what type it currently is and just specify what type we want it to be.
|
In general the change is good I think, but I want to have good clarity on the casting situation. We should document this in comments, once I understand it. |
Sure, I clarified your queries above. |
|
Please let me know if you have further doubts in the implementation. I will be happy to clarify them. |
certik
left a comment
certik
left a comment
There was a problem hiding this comment.
I think that this is fine. Would you mind submitting this PR against LFortran as well?
|
Thank you so much for the review.
Sure. Is it fine to merge this PR currently? |
|
Yes, you can merge it. |
2 participants
fixes #2349