[PATCH] D152538: `G_ATOMICRMW_NAND` failure in legalizer

[niwin anto via Phabricator via llvm-commits]

niwinanto added inline comments.


================
Comment at: llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp:4197
+      AAMDNodes(), nullptr, SyncScope::System, AtomicOrdering::Unordered);
+
+  auto MIBAtomicRMWNand = B.buildAtomicRMWNand(OldValRes, Addr, Val, *MMO);
----------------
arsenm wrote:
> Ideally would have some defs for it
Yes, but I don't find any def for `MachineMemOperand` and this is how other tests are also using it.


================
Comment at: llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp:4198
+
+  auto MIBAtomicRMWNand = B.buildAtomicRMWNand(OldValRes, Addr, Val, *MMO);
+
----------------
arsenm wrote:
> can just specify the return type here
You meant `auto MIBAtomicRMWNand` to `MachineInstrBuilder MIBAtomicRMWNand`, or?


================
Comment at: llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp:4211
+  // Check
+  EXPECT_TRUE(CheckMachineFunction(*MF, CheckStr)) << *MF;
+}
----------------
arsenm wrote:
> This should really fail with use of undefined value
I don't really get this. What do you mean?


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

https://reviews.llvm.org/D152538