x86: preserve flags when folding atomic operations

[jfb]

Summary

D4796 taught LLVM to fold some atomic integer operations into a single
instruction. The pattern was unaware that the instructions clobbered
flags.

This patch adds the missing EFLAGS definition.

Floating point operations don't set flags, the fadd optimization a few
lines below is therefore correct. The same applies for surrounding
load/store optimizations.

[dschuff]

The change LGTM; it's not clear what you mean by "the subsequent fadd optimization" in the description without some more context.

lib/Target/X86/X86InstrCompiler.td

def NAME#64mi32 : I<0, Pseudo, (outs), (ins i64mem:$dst, i64i32imm:$src),
         "#BINOP "#NAME#"64mi32 PSEUDO!",
         [(atomic_store_64 addr:$dst, (op
             (atomic_load_64 addr:$dst), (i64immSExt32:$src)))]>;
     def NAME#64mr : I<0, Pseudo, (outs), (ins i64mem:$dst, GR64:$src),
         "#BINOP "#NAME#"64mr PSEUDO!",
         [(atomic_store_64 addr:$dst, (op
             (atomic_load_64 addr:$dst), GR64:$src))]>;
 }
+let Defs = [EFLAGS] in {
   defm RELEASE_ADD : RELEASE_BINOP_MI<add>;
   defm RELEASE_AND : RELEASE_BINOP_MI<and>;
   defm RELEASE_OR  : RELEASE_BINOP_MI<or>;
   defm RELEASE_XOR : RELEASE_BINOP_MI<xor>;
   // Note: we don't deal with sub, because substractions of constants are
-// optimized into additions before this code can run
+  //       optimized into additions before this code can run.
+}
 
 // Same as above, but for floating-point.
 // FIXME: imm version.
 // FIXME: Version that doesn't clobber $src, using AVX's VADDSS.
 // FIXME: This could also handle SIMD operations with *ps and *pd instructions.
 let usesCustomInserter = 1 in {
 multiclass RELEASE_FP_BINOP_MI<SDNode op> {
     def NAME#32mr : I<0, Pseudo, (outs), (ins i32mem:$dst, FR32:$src),

test/CodeGen/X86/atomic-flags.ll

+; RUN: llc < %s -mtriple=x86_64-unknown-unknown -verify-machineinstrs | FileCheck %s
+; RUN: llc < %s -mtriple=i686-unknown-unknown -verify-machineinstrs | FileCheck %s
+
+; Make sure that flags are properly preserved despite atomic optimizations.
+
+define i32 @atomic_and_flags(i8* %p, i32 %a, i32 %b) {
+; CHECK-LABEL: atomic_and_flags:
+
+  ; Generate flags value, and use it.
+  ; CHECK:      cmpl
+  ; CHECK-NEXT: jne
+  %cmp = icmp eq i32 %a, %b
+  br i1 %cmp, label %L1, label %L2
+
+L1:
+  ; The following pattern will get folded.
+  ; CHECK: addb
+  %1 = load atomic i8, i8* %p seq_cst, align 1
+  %2 = add i8 %1, 2
+  store atomic i8 %2, i8* %p release, align 1
+
+  ; Use the comparison result again. We need to rematerialize the comparison
+  ; somehow. This test checks that cmpl gets emitted again, but any
+  ; rematerialization would work (the optimizer used to clobber the flags with
+  ; the add).
+  ; CHECK-NEXT: cmpl
+  ; CHECK-NEXT: jne
+  br i1 %cmp, label %L3, label %L4
+
+L2:
+  ret i32 2
+
+L3:
+  ret i32 3
+
+L4:
+  ret i32 4
+}