- Notifications
You must be signed in to change notification settings - Fork 15.3k
Description
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_2Here 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.