[PATCH] D142166: [X86] Make `shouldExpandLogicAtomicRMWInIR` able to match both operands.

[Phoebe Wang via Phabricator via llvm-commits]

pengfei accepted this revision.
pengfei added a comment.
This revision is now accepted and ready to land.

LGTM with some nits.



================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:31534-31536
+  // This is a redundant AND, it should get cleaned up elsewhere.
+  if (AI == I->getOperand(OtherIdx))
+    return AtomicExpansionKind::CmpXChg;
----------------
Do you have a test for it? I suspect it's dead code. Maybe just use assert?


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:31540-31541
   if (BitChange.second == ConstantBit || BitChange.second == NotConstantBit) {
     auto *C1 = dyn_cast<ConstantInt>(AI->getValOperand());
     assert(C1 != nullptr);
+    auto *C2 = dyn_cast<ConstantInt>(I->getOperand(OtherIdx));
----------------
This can trun into `auto *C1 = cast<ConstantInt>(AI->getValOperand());`.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:31610-31612
+    auto *C =
+        dyn_cast<ConstantInt>(I->getOperand(I->getOperand(0) == AI ? 1 : 0));
     assert(C != nullptr);
----------------
ditto.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142166