[PATCH] D11382: x86 atomic: optimize a.store(reg op a.load(acquire), release)

[JF Bastien]

jfb marked an inline comment as done.

================
Comment at: lib/Target/X86/X86InstrCompiler.td:765
@@ -763,2 +764,3 @@
+        "#BINOP "#NAME#"8mi PSEUDO!",
         [(atomic_store_8 addr:$dst, (!cast<PatFrag>(op)
             (atomic_load_8 addr:$dst), (i8 imm:$src)))]>;
----------------
jfb wrote:
> pete wrote:
> > This was there prior to your change, but i wonder if we should have a later patch (by you or me or anyone else) to consider removing this cast.  We can do so by taking 'SDNode op' instead of a string.  For example
> > 
> > diff --git a/lib/Target/X86/X86InstrCompiler.td b/lib/Target/X86/X86InstrCompiler.td
> > index 49dc318..9168713 100644
> > --- a/lib/Target/X86/X86InstrCompiler.td
> > +++ b/lib/Target/X86/X86InstrCompiler.td
> > @@ -757,26 +757,26 @@ defm LXADD : ATOMIC_LOAD_BINOP<0xc0, 0xc1, "xadd", "atomic_load_add",
> >   * extremely late to prevent them from being accidentally reordered in the backend
> >   * (see below the RELEASE_MOV* / ACQUIRE_MOV* pseudo-instructions)
> >   */
> > -multiclass RELEASE_BINOP_MI<string op> {
> > +multiclass RELEASE_BINOP_MI<SDNode op> {
> >      def NAME#8mi : I<0, Pseudo, (outs), (ins i8mem:$dst, i8imm:$src),
> >          "#RELEASE_BINOP PSEUDO!",
> > -        [(atomic_store_8 addr:$dst, (!cast<PatFrag>(op)
> > +        [(atomic_store_8 addr:$dst, (op
> >              (atomic_load_8 addr:$dst), (i8 imm:$src)))]>;
> >      // NAME#16 is not generated as 16-bit arithmetic instructions are considered
> >      // costly and avoided as far as possible by this backend anyway
> >      def NAME#32mi : I<0, Pseudo, (outs), (ins i32mem:$dst, i32imm:$src),
> >          "#RELEASE_BINOP PSEUDO!",
> > -        [(atomic_store_32 addr:$dst, (!cast<PatFrag>(op)
> > +        [(atomic_store_32 addr:$dst, (op
> >              (atomic_load_32 addr:$dst), (i32 imm:$src)))]>;
> >      def NAME#64mi32 : I<0, Pseudo, (outs), (ins i64mem:$dst, i64i32imm:$src),
> >          "#RELEASE_BINOP PSEUDO!",
> > -        [(atomic_store_64 addr:$dst, (!cast<PatFrag>(op)
> > +        [(atomic_store_64 addr:$dst, (op
> >              (atomic_load_64 addr:$dst), (i64immSExt32:$src)))]>;
> >  }
> > -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">;
> > +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>;
> I can send a follow-up.
Done in D11788.


Repository:
  rL LLVM

http://reviews.llvm.org/D11382