Add a MIR pass to lower 128-bit operators to lang item calls #46093
Conversation
Runs only with `-Z lower_128bit_ops` since it's not hooked into targets yet.
kennytm
added
the
S-waiting-on-review
| // END RUST SOURCE | ||
|
|
||
| // START rustc.test_signed.Lower128Bit.after.mir | ||
| // _2 = const i128_addo(_1, const 1i128) -> bb10; |
There was a problem hiding this comment.
This test should include the statements dealing with the tuple projections (i.e. the branch on _2.1 and the _1 = _2.0).
| let ty = lhs.ty(local_decls, tcx); | ||
| if let Some(is_signed) = sign_of_128bit(&ty) { | ||
| if let Some(item) = item_for_op(bin_op, is_signed) { | ||
| return Some(tcx.require_lang_item(item)) |
There was a problem hiding this comment.
Since lang-items aren’t yet type-checked, could you add an assertion of some sort that checks whether the function really has the expected type?
#9307 is a long standing issue regarding that, and I would rather have an assertion in an easy to track location rather than right before trans.
There was a problem hiding this comment.
Ok, the next update generates an error like
thread 'rustc' panicked at 'assertion failed: `(left == right)`
left: `[i128, i128, (i128, u32)]`,
right: `[i128, i128, (i128, bool)]`: lang item _ZN23lower_128bit_debug_test9i128_addo', src\librustc_mir\transform\lower_128bit.rs:115:4
|
This looks okay to me, although yet again I’m slightly disappointed the I feel that r? @nagisa |
nagisa
added
S-waiting-on-author
As part of doing so, add more lang items instead of passing u128 to the i128 ones where it doesn't matter in twos-complement.
* The overflow-checking shift items need to take a full 128-bit type, since they need to be able to detect idiocy like `1i128 << (1u128 << 127)` * The unchecked ones just take u32, like the `*_sh?` methods in core * Because shift-by-anything is allowed, cast into a new local for every shift
| (BinOp::Sub, false) => (LangItem::U128SuboFnLangItem, RhsKind::Unchanged), | ||
| (BinOp::Mul, true) => (LangItem::I128MuloFnLangItem, RhsKind::Unchanged), | ||
| (BinOp::Mul, false) => (LangItem::U128MuloFnLangItem, RhsKind::Unchanged), | ||
| (BinOp::Shl, true) => (LangItem::I128ShloFnLangItem, RhsKind::ForceU128), |
There was a problem hiding this comment.
Why specifically u128 for overflowing shifts? Is there an already-existing implementation that takes u128 as a shift count (I couldn’t find it in compiler-builtins)?
There was a problem hiding this comment.
It's unfortunately necessary to preserve the current behaviour of
1i128 << (1u128 << 127)https://play.rust-lang.org/?gist=78f8381aac3ef89febcb48e56194e948&version=nightly
Any smaller type and the fact the shift is out of range would be lost.
|
@bors r+ |
|
📌 Commit 42208c1 has been approved by |
kennytm
added
S-waiting-on-bors
Add a MIR pass to lower 128-bit operators to lang item calls Runs only with `-Z lower_128bit_ops` since it's not hooked into targets yet. This isn't really useful on its own, but the declarations for the lang items need to be in the compiler before compiler-builtins can be updated to define them, so this is part 1 of at least 3. cc #45676 @est31 @nagisa
|
☀️ Test successful - status-appveyor, status-travis |
Runs only with
-Z lower_128bit_opssince it's not hooked into targets yet.This isn't really useful on its own, but the declarations for the lang items need to be in the compiler before compiler-builtins can be updated to define them, so this is part 1 of at least 3.
cc #45676 @est31 @nagisa