[PATCH] D35319: LSE Atomics reorg - Part I

[Tim Northover via Phabricator via llvm-commits]

t.p.northover added inline comments.


================
Comment at: lib/Target/AArch64/AArch64InstrFormats.td:9401
 multiclass CompareAndSwap<bits<1> Acq, bits<1> Rel, string order> {
-  let Sz = 0b00, Acq = Acq, Rel = Rel in def b : BaseCAS<order, "b", GPR32>;
-  let Sz = 0b01, Acq = Acq, Rel = Rel in def h : BaseCAS<order, "h", GPR32>;
-  let Sz = 0b10, Acq = Acq, Rel = Rel in def s : BaseCAS<order, "", GPR32>;
-  let Sz = 0b11, Acq = Acq, Rel = Rel in def d : BaseCAS<order, "", GPR64>;
+  let Sz = 0b00, Acq = Acq, Rel = Rel in def B : BaseCAS<order, "b", GPR32>;
+  let Sz = 0b01, Acq = Acq, Rel = Rel in def H : BaseCAS<order, "h", GPR32>;
----------------
steleman wrote:
> t.p.northover wrote:
> > I'm fine with this change, but it should probably be committed before as a separate "NFC" rename. In fact, feel free to do that any time you like.
> Won't this complicate things, though?
> 
> If I commit this change as a separate - and "before" change, I'll still have to change all the instruction mnemonics in the existing Pats, and in the AArch64DeadRegister blacklist.
> 
> It seems like a lot of code churn.
> 
OK, I won't insist.


================
Comment at: lib/Target/AArch64/AArch64InstrFormats.td:9517
+
+multiclass STOPregister_patterns_ord<string inst, string suffix,
+                                     string op, string size,
----------------
steleman wrote:
> t.p.northover wrote:
> > These `STOP` patterns appear to be unused.
> Yes, they are unused for now. But I plan on adding the ST<OP> instructions as well.
> 
> They need ISD NodeTypes, though, as ISD::ATOMIC_STORE_ADD, ISD::ATOMIC_STORE_SUB,
> etc, aren't defined in include/llvm/CodeGen/ISDOpcodes.h. Which is why I'm saving
> this change for a subsequent changeset.
> 
> 
Then that would be a separate patch.

Fair warning: I strongly suspect it's unnecessary and should be automatically handled by the DeadDefinitions pass. ATOMIC_STORE_ADD is nothing but a normal ATOMIC_LOAD_ADD where the definition is dead, which is exactly what that pass is supposed to handle.


Repository:
  rL LLVM

https://reviews.llvm.org/D35319