Skip to content
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

[Lang] Fix integral type promotion rules (e.g., u8 + u8 now leads to u8 instead of i32) #4789

Merged
merged 3 commits into from
Apr 15, 2022

Conversation

yuanming-hu
Copy link
Member

@yuanming-hu yuanming-hu commented Apr 14, 2022

Related issue = closes #4613
Related issue = #2196

It's interesting that in C++ i8 + i8 = i32.

Type promotion table:
Format: a + b = old_type -> new_type. Only some are changed.

f32 + f32 = f32
f32 + f64 = f64
f32 + i8 = f32
f32 + i16 = f32
f32 + i32 = f32
f32 + i64 = f32
f32 + u8 = f32
f32 + u16 = f32
f32 + u32 = f32
f32 + u64 = f32
f64 + f32 = f64
f64 + f64 = f64
f64 + i8 = f64
f64 + i16 = f64
f64 + i32 = f64
f64 + i64 = f64
f64 + u8 = f64
f64 + u16 = f64
f64 + u32 = f64
f64 + u64 = f64
i8 + f32 = f32
i8 + f64 = f64
i8 + i8 = i32 -> i8
i8 + i16 = i32 -> i16
i8 + i32 = i32
i8 + i64 = i64
i8 + u8 = i32 -> u8
i8 + u16 = i32 -> u16
i8 + u32 = u32
i8 + u64 = u64
i16 + f32 = f32
i16 + f64 = f64
i16 + i8 = i32 -> i16
i16 + i16 = i32 -> i16
i16 + i32 = i32
i16 + i64 = i64
i16 + u8 = i32 -> i16
i16 + u16 = i32 -> u16
i16 + u32 = u32
i16 + u64 = u64
i32 + f32 = f32
i32 + f64 = f64
i32 + i8 = i32
i32 + i16 = i32
i32 + i32 = i32
i32 + i64 = i64
i32 + u8 = i32
i32 + u16 = i32
i32 + u32 = u32
i32 + u64 = u64
i64 + f32 = f32
i64 + f64 = f64
i64 + i8 = i64
i64 + i16 = i64
i64 + i32 = i64
i64 + i64 = i64
i64 + u8 = i64
i64 + u16 = i64
i64 + u32 = i64
i64 + u64 = u64
u8 + f32 = f32
u8 + f64 = f64
u8 + i8 = i32 -> u8
u8 + i16 = i32 -> i16
u8 + i32 = i32
u8 + i64 = i64
u8 + u8 = i32 -> u8
u8 + u16 = i32 -> u16
u8 + u32 = u32
u8 + u64 = u64
u16 + f32 = f32
u16 + f64 = f64
u16 + i8 = i32 -> u16
u16 + i16 = i32 -> u16
u16 + i32 = i32
u16 + i64 = i64
u16 + u8 = i32 -> u16
u16 + u16 = i32 -> u16
u16 + u32 = u32
u16 + u64 = u64
u32 + f32 = f32
u32 + f64 = f64
u32 + i8 = u32
u32 + i16 = u32
u32 + i32 = u32
u32 + i64 = i64
u32 + u8 = u32
u32 + u16 = u32
u32 + u32 = u32
u32 + u64 = u64
u64 + f32 = f32
u64 + f64 = f64
u64 + i8 = u64
u64 + i16 = u64
u64 + i32 = u64
u64 + i64 = u64
u64 + u8 = u64
u64 + u16 = u64
u64 + u32 = u64
u64 + u64 = u64

@netlify
Copy link

netlify bot commented Apr 14, 2022

Deploy Preview for docsite-preview canceled.

Name Link
🔨 Latest commit 9addcdc
🔍 Latest deploy log https://app.netlify.com/sites/docsite-preview/deploys/625840cd1b9a0a00088421c3

@yuanming-hu yuanming-hu changed the title [Lang] Fix type promotion rules [Lang] Fix type promotion rules (u8 + u8 now leads to u8) Apr 14, 2022
@yuanming-hu yuanming-hu changed the title [Lang] Fix type promotion rules (u8 + u8 now leads to u8) [Lang] Fix type promotion rules (e.g., u8 + u8 now leads to u8) Apr 14, 2022
@yuanming-hu yuanming-hu changed the title [Lang] Fix type promotion rules (e.g., u8 + u8 now leads to u8) [Lang] Fix integral operation type promotion rules (e.g., u8 + u8 now leads to u8) Apr 14, 2022
@yuanming-hu yuanming-hu changed the title [Lang] Fix integral operation type promotion rules (e.g., u8 + u8 now leads to u8) [Lang] Fix integral type promotion rules (e.g., u8 + u8 now leads to u8 instead of i32) Apr 14, 2022
@yuanming-hu yuanming-hu requested a review from strongoier April 14, 2022 16:35
Copy link
Contributor

@strongoier strongoier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!!

I was previously stuck at this issue because I thought we must follow C in arithmetic operations. However this is not a necessary assumption...

BTW I think we need to update the doc as well:

Taichi follows the [implicit conversion rules](https://en.cppreference.com/w/c/language/conversion) for the C programming language and implicitly casts operands in a [binary operation](https://en.wikipedia.org/wiki/Binary_operation) into a *common type* if the operation involves different data types. Following are two most straightforward rules for determining the common type in a binary operation:

taichi/ir/type_factory.cpp Show resolved Hide resolved
@yuanming-hu
Copy link
Member Author

Thank you @strongoier!

I thought we must follow C in arithmetic operations. However this is not a necessary assumption...

I was also stuck a bit here, until I realized that LLVM uses a similar rule as ours:

char add(char a, char b) {
    return a + b;
}
define signext i8 @add(i8 signext %0, i8 signext %1) local_unnamed_addr #0 {
  %3 = add i8 %1, %0
  ret i8 %3
}

Could you help create an issue for the doc update?

One more thing I would like to point out:

Currently, all binary operators follow the current promotion rule. But there are exceptions. For example, i8 << u32 may end up with i8, but currently it ends up with u32. We need to reach a consensus here: should we simply take the left-hand side type as the shift operator outputs?

@yuanming-hu yuanming-hu merged commit 6df23cb into taichi-dev:master Apr 15, 2022
@yuanming-hu yuanming-hu deleted the fix-type-promotion branch April 15, 2022 05:21
@strongoier
Copy link
Contributor

Could you help create an issue for the doc update?

#4793.

We need to reach a consensus here: should we simply take the left-hand side type as the shift operator outputs?

I agree. #4795.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Why are there precision warnings when no type changes (target = u8, value = u8)?
2 participants