sparker-arm opened PR #3070 from simd-extmul-aarch64
to main
:
Lower simd_extmul_[low/high][signed/unsigned] to [s|u]widen inputs to
an imul node.This also reorganises the aarch64 'long mul' operations and their assembly.
Copyright (c) 2021, Arm Limited.
<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
sparker-arm updated PR #3070 from simd-extmul-aarch64
to main
.
akirilov-arm requested cfallin for a review on PR #3070.
alexcrichton submitted PR review.
alexcrichton created PR review comment:
It seems like this and
Umlal8
aren't actually generated in any lowerings (unless I'm missing something), but do you want to leave these in for completeness with possible future lowerings?
alexcrichton submitted PR review.
alexcrichton created PR review comment:
One possible way to structure this is to perhaps check for
is_vector
and i64x2 early on at the top of this block with early-returns if they match, and if that fails all the remaining cases share the logic oflet rn = put_input_in_reg(ctx, inputs[0], NarrowValueMode::None); let rm = put_input_in_reg(ctx, inputs[1], NarrowValueMode::None); let rd = get_output_reg(ctx, outputs[0]).only_reg().unwrap();
as a prefix.
Just a thought though, happy to defer to you who work on this much more than I!
sparker-arm submitted PR review.
sparker-arm created PR review comment:
Yes, exactly - thought it would be weird to just port the 32-bit version.
sparker-arm submitted PR review.
sparker-arm created PR review comment:
So even though I don't like the duplication, or how much conditional stuff is going on here, we would only be able to factor out one collection of these calls as the I128 case uses, ever so slightly, different calls to handle multiple registers. So I think I prefer the clarity.
cfallin submitted PR review.
cfallin created PR review comment:
Can we add a doc-comment here describing the return tuple? E.g. it's not clear to me reading at this point what the
bool
denotes, until I notice the low/high pattern below in the implementation.
cfallin submitted PR review.
sparker-arm updated PR #3070 from simd-extmul-aarch64
to main
.
cfallin submitted PR review.
cfallin merged PR #3070.
Last updated: Dec 23 2024 at 12:05 UTC