Skip to content

[M68k] Miscompile: COPY and copyPhysReg() do not respect live CCR register #152816

@dansalvato

Description

@dansalvato

The easiest way to reproduce this is with the following IR:

target triple = "m68k-freestanding" define i8 @atomicrmw_add_i8(i8 %val, ptr %ptr) { %old = atomicrmw add ptr %ptr, i8 %val monotonic ret i8 %old }

Build with -mcpu=M68020 and notice this block in the result:

.LBB0_1: move.b%d2, %d3 add.b%d1, %d3 cas.b %d0, %d3, (%a0) move.b%d0, %d3 sub.b%d2, %d3 seq%d2 and.b#1, %d2 cmpi.b#0, %d2 move.b%d0, %d2 ; <------ Kills CCR beq.LBB0_1 bra.LBB0_2

Here is the MC:

 bb.1.atomicrmw.start: successors: %bb.2(0x04000000), %bb.1(0x7c000000) liveins: $a0, $bd0, $bd1, $bd2 $bd3 = COPY $bd2 $bd3 = ADD8dd killed $bd3, $bd1, implicit-def dead $ccr $bd0 = CASARI8 killed $bd0, killed $bd3, $a0 :: (load store monotonic monotonic (s8) on %ir.ptr) $bd3 = COPY $bd0 dead $bd3 = SUB8dd killed $bd3, killed $bd2, implicit-def $ccr $bd2 = SETd8eq implicit killed $ccr $bd2 = AND8di killed $bd2, 1, implicit-def dead $ccr CMP8di 0, killed $bd2, implicit-def $ccr $bd2 = COPY $bd0 Beq8 %bb.1, implicit killed $ccr BRA8 %bb.2 

I wrote my own patch that mitigates this, but it's more of a workaround. In copyPhysReg(), if CCR is live and the destination is a data register, I back up the CCR and restore it after the copy.

In my opinion, this issue is severe enough that it warrants merging a workaround patch, but I'd like a second opinion before I submit something.

The workaround could be improved if it's somehow possible to re-allocate the destination to an address register, because copying to address registers doesn't affect CCR. But I think the preferable solution would be to prevent COPY from ever being emitted in locations where the CCR is live.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions