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

[PowerPC] Missing sign extension in expansion of sub-word atomic max #61882

Closed
Amanieu opened this issue Apr 1, 2023 · 2 comments
Closed

[PowerPC] Missing sign extension in expansion of sub-word atomic max #61882

Amanieu opened this issue Apr 1, 2023 · 2 comments
Assignees

Comments

@Amanieu
Copy link
Contributor

Amanieu commented Apr 1, 2023

IR

define void @foo(ptr %a, i32 %x) {
  %val = trunc i32 %x to i8
  %1 = atomicrmw max ptr %a, i8 %val seq_cst
  ret void
}

Asm (-march=ppc32)

foo:                                    # @foo
        rlwinm 5, 3, 3, 27, 28
        li 6, 255
        xori 5, 5, 24
        slw 7, 4, 5
        slw 6, 6, 5
        rlwinm 3, 3, 0, 0, 29
        and 7, 7, 6
        sync
.LBB0_1:                                # =>This Inner Loop Header: Depth=1
        lwarx 8, 0, 3
        and 9, 8, 6
        srw 9, 9, 5
        extsb 9, 9
        cmpw    9, 4 # <== Register 4 used here without being sign-extended
        bgt     0, .LBB0_3
        andc 8, 8, 6
        or 8, 7, 8
        stwcx. 8, 0, 3
        bne     0, .LBB0_1
.LBB0_3:
        lwsync
        blr

The problem

The second parameter of the function (%x, register 4) needs to be sign-extended before being compared against the value extracted from the loaded word.

Downstream bug: rust-lang/rust#100650

@llvmbot
Copy link
Member

llvmbot commented Apr 1, 2023

@llvm/issue-subscribers-backend-powerpc

@bzEq bzEq self-assigned this Apr 4, 2023
@bzEq
Copy link
Collaborator

bzEq commented Apr 5, 2023

Posted https://reviews.llvm.org/D147594.

@bzEq bzEq closed this as completed in eee024b Apr 14, 2023
flemairen6 pushed a commit to Xilinx/llvm-project that referenced this issue May 10, 2023
After performing signed extension, we update the register in MI. We should also update `incr` register which is tracking the register in `MI`.

Fixes llvm#61882.

Reviewed By: #powerpc, shchenz

Differential Revision: https://reviews.llvm.org/D147594
veselypeta pushed a commit to veselypeta/cherillvm that referenced this issue Aug 21, 2024
After performing signed extension, we update the register in MI. We should also update `incr` register which is tracking the register in `MI`.

Fixes llvm/llvm-project#61882.

Reviewed By: #powerpc, shchenz

Differential Revision: https://reviews.llvm.org/D147594
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants