cranelift(aarch64): share base+index across offset loads via uimm12 amode

[darmie]

When several loads share base + (extended index) and differ only by a
uimm12-scaled offset (e.g. fields read through a computed index), AArch64 amode
selection emitted a separate add base, #offset per load, defeating CSE of the
shared base + index.

This folds the offset into the load's immediate and keeps base + index as one
value, so a single add base, index, uxtw is materialized and reused. A zero
offset keeps the single-instruction RegExtended form, so it is
neutral-or-better: a cluster of N such loads drops N address-adds to 1, a lone
load is unchanged.

Tested: new aarch64 precise-output filetest + a stack round-trip runtest
(aarch64/x86_64/s390x); the full aarch64 filetest and runtest suites pass.

[alexcrichton]

Thanks for this! I'm wary of the duplication being added here though, and amode rules are some of the most sensitive in Cranelift so we ideally want as little duplication as possible. For example the offset=0 rules are duplicates of above rules, and even using AMode.UnsignedOffset is a duplicate of prior rules as well. Would it be possible to shuffle rules around and add if-let clauses perhaps to tweak existing rules? For example rules could use if-let or some sort of extraction matcher to ensure that offset is 0. Either that or the rule using AMode.UnsignedOffset could be given higher priority.

[darmie]

For example the offset=0 rules are duplicates of above rules, and even using AMode.UnsignedOffset is a duplicate of prior rules as well. Would it be possible to shuffle rules around and add if-let clauses perhaps to tweak existing rules? For example rules could use if-let or some sort of extraction matcher to ensure that offset is 0. Either that or the rule using AMode.UnsignedOffset could be given higher priority.

I have removed the duplication. Also added uimm12_scaled_nonzero_from_i64 (+ its verifier spec) so rules 8/9's UnsignedOffset only matches non-zero offsets and takes higher priority than 4/5; a zero offset falls through to the existing RegExtended rules naturally.

[github-actions]

Subscribe to Label Action

cc @cfallin, @fitzgen

Details

This issue or pull request has been labeled: "cranelift", "cranelift:area:aarch64", "isle"

Thus the following users have been cc'd because of the following labels:

• cfallin: isle
• fitzgen: isle

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

[alexcrichton]

This helps remove some duplication, yeah, but this is still effectively a duplication of rule (2), however, and it's also a duplication of 3/4/5 in terms of matching pattern. My thinking is that what you want can be achieved by raising the priority of rule 2 because the premise here is that the static offset should have the highest priority for getting folded into the instruction itself while leaving all other computations in a CSE-able form.