[PATCH] D58632: [X86] Improve lowering of idemptotent RMW operations

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:25455
+  // TODO: push more cases through this path. 
+  if (auto C = dyn_cast<ConstantInt>(AI->getValOperand()))
+    if (AI->getOperation() == AtomicRMWInst::Or && C->isZero() &&
----------------
Should that be "auto *C"? I think we try to keep the * on pointers.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:25995
+      DAG.getRegister(0, MVT::i64),                         // Index
+      DAG.getTargetConstant(SPOffset, DL, MVT::i32), // Disp
+      DAG.getRegister(0, MVT::i32),                         // Segment.
----------------
Line comments up?


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:25999
+      Chain};
+    SDNode *Res = DAG.getMachineNode(X86::LOCK_OR64mi32, DL, MVT::Other, Ops);
+    return SDValue(Res, 0);
----------------
Shouldn't this be mi8 for a shorter encoding?

Does 64-bit really need a 64-bit access or can we just use a 32-bit access?


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:26081
+  if (Opc == ISD::ATOMIC_LOAD_OR &&
+      isa<ConstantSDNode>(RHS) &&
+      cast<ConstantSDNode>(RHS)->isNullValue()) {
----------------
I think you can use isNullConstant


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58632/new/

https://reviews.llvm.org/D58632