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.

Sign up for GitHub

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

Jump to bottom

Incorrect codegen for unaligned load/store (ARM NEON) #59081

Closed
Nugine opened this issue Nov 19, 2022 · 5 comments · Fixed by #106984
Closed

Incorrect codegen for unaligned load/store (ARM NEON) #59081

Nugine opened this issue Nov 19, 2022 · 5 comments · Fixed by #106984
Assignees
@nikic
Labels
backend:ARM miscompilation release:backport release:cherry-pick-failed
Milestone
LLVM 19.X Release

Comments

@Nugine
Copy link

Nugine commented Nov 19, 2022

https://clang.godbolt.org/z/nEfsT961d

#include <arm_neon.h>
#include <stdint.h>

void double32(uint8_t* data) {
    uint8x16x2_t x = vld1q_u8_x2(data);
    uint8x16_t y0 = vaddq_u8(x.val[0], x.val[0]);
    uint8x16_t y1 = vaddq_u8(x.val[1], x.val[1]);
    uint8x16x2_t y = {y0, y1};
    vst1q_u8_x2(data, y);
}
double32:
        vld1.8  {d16, d17, d18, d19}, [r0:256] ; <- should be [r0]
        vshl.i8 q11, q9, #1
        vshl.i8 q10, q8, #1
        vst1.8  {d20, d21, d22, d23}, [r0:256] ; <- should be [r0]
        bx      lr

https://developer.arm.com/documentation/den0018/a/NEON-Instruction-Set-Architecture/Alignment

It seems that there is no way to generate unaligned load/store instructions.
@llvmbot
Copy link
Member

llvmbot commented Nov 19, 2022

@llvm/issue-subscribers-backend-arm

@Nugine Nugine mentioned this issue Nov 19, 2022
Open
@Amanieu Amanieu mentioned this issue Nov 29, 2023
Open
@nikic nikic self-assigned this Sep 2, 2024
nikic added a commit to nikic/llvm-project that referenced this issue Sep 2, 2024
@nikic
[ARM] Do not assume alignment in vld1xN and vst1xN intrinsics
6c09e2c 
These intrinsics currently assume natural alignment. Instead,
respect the alignment attribute on the intrinsic. Teach InstCombine
to improve that alignment.

If desired I could also adjust the clang frontend to add alignment
annotations equivalent to the previous behavior, but I don't see
any indication that such an assumption is correct in the ARM
intrinsics docs.

Fixes llvm#59081.
nikic added a commit to nikic/llvm-project that referenced this issue Sep 2, 2024
@nikic
[ARM] Do not assume alignment in vld1xN and vst1xN intrinsics
83e3409 
These intrinsics currently assume natural alignment. Instead,
respect the alignment attribute on the intrinsic. Teach InstCombine
to improve that alignment.

If desired I could also adjust the clang frontend to add alignment
annotations equivalent to the previous behavior, but I don't see
any indication that such an assumption is correct in the ARM
intrinsics docs.

Fixes llvm#59081.
@nikic nikic mentioned this issue Sep 2, 2024
Merged
nikic added a commit that referenced this issue Sep 5, 2024
@nikic
[ARM] Do not assume alignment in vld1xN and vst1xN intrinsics (#106984)
a7697c8 
These intrinsics currently assume natural alignment. Instead, respect
the alignment attribute on the intrinsic. Teach InstCombine to improve
that alignment.

If desired I could also adjust the clang frontend to add alignment
annotations equivalent to the previous behavior, but I don't see any
indication that such an assumption is correct in the ARM intrinsics
docs.

Fixes #59081.
@Amanieu
Copy link
Contributor

Amanieu commented Dec 18, 2024

/cherry-pick a7697c8

@llvmbot
Copy link
Member

llvmbot commented Dec 18, 2024

/cherry-pick a7697c8

Error: Command failed due to missing milestone.

@Amanieu Amanieu added this to the LLVM 19.X Release milestone Dec 18, 2024
@github-project-automation github-project-automation bot moved this to Needs Triage in LLVM Release Status Dec 18, 2024
@Amanieu
Copy link
Contributor

Amanieu commented Dec 18, 2024

/cherry-pick a7697c8

@llvmbot
Copy link
Member

llvmbot commented Dec 18, 2024

Failed to cherry-pick: a7697c8

https://github.com/llvm/llvm-project/actions/runs/12403059897

Please manually backport the fix and push it to your github fork. Once this is done, please create a pull request

@tru tru moved this from Needs Triage to Done in LLVM Release Status Jan 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nikic nikic

Labels
backend:ARM miscompilation release:backport release:cherry-pick-failed
Projects
LLVM Release Status
Status: Done
Milestone
LLVM 19.X Release
Development

Successfully merging a pull request may close this issue.

[ARM] Do not assume alignment in vld1xN and vst1xN intrinsics nikic/llvm-project
6 participants
@nikic @Amanieu @EugeneZelenko @thesamesam @Nugine @llvmbot