[PATCH] D35319: LSE Atomics reorg - Part I

[Tim Northover via Phabricator via llvm-commits]

t.p.northover added a comment.

> I implemented the memory ordering semantics for all the LSE Atomics with Intrinsics.

It's unnecessary to add intrinsics to handle this, let alone get C++ code involved. We've got exactly the ISD::AtomicRMW node we want and it already has all the information we need attached.

If you add a fragment like this to include/llvm/Target/TargetSelectionDAG.td:

  multiclass binary_atomic_op_ord {
    def #NAME#_monotonic : PatFrag<(ops node:$ptr, node:$val),
          (!cast<SDNode>(#NAME) node:$ptr, node:$val), [{
        return cast<AtomicSDNode>(N)->getOrdering() == AtomicOrdering::Monotonic;
      }]>;
     [...]
  }

and adapt the definition of `binary_atomic_op` to use it you can refer directly to things like `atomic_load_add_8_acquire` from TableGen. A few more multiclasses to reduce copy/paste in AArch64InstrAtomics.td along the lines of:

  multiclass LDOPregister_patterns_ord_dag<string inst, string suffix, string op,
                                       string size, dag SrcRHS, dag DstRHS> {
    def : Pat<(!cast<SDNode>(op#"_"#size#"_monotonic") GPR64sp:$Rn, SrcRHS),
              (!cast<Instruction>(inst#suffix) DstRHS, GPR64sp:$Rn)>;
    [...]
  }
  
  multiclass LDOPregister_patterns<string inst, string op> {
    defm : LDOPregister_patterns_ord<inst, "d", op, "64", (i64 GPR64:$Rm)>; // 64-bit
    [...]
  }
  
  defm : LDOPregister_patterns<"LDADD", "atomic_load_add">;
  [...]

and the job's a goodun.

> This is the reason why the instruction definitions were expanded to explicitly describe each and every single instruction - as opposed to using the original multiclass design: a different Intrinsic need to be passed to the Instruction Definition depending on register size, and memory ordering. I do not think it is possible to accomplish this particular design with a multiclass.

Even if we wanted the intrinsic-based design (I definitely don't) it would be better to put separate patterns elsewhere than to contort the existing instruction definitions to accommodate it. And splitting the ST* instructions is still a bad idea either way.


Repository:
  rL LLVM

https://reviews.llvm.org/D35319