[PATCH] D152538: `G_ATOMICRMW_NAND` failure in legalizer
Matt Arsenault via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sat Jun 10 04:40:03 PDT 2023
arsenm added inline comments.
================
Comment at: llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp:4207
+ auto CheckStr = R"(
+ CHECK: [[ATMNAND:%[0-9]+]]:_(s64) = G_ATOMICRMW_NAND
+ )";
----------------
Also check the inputs
================
Comment at: llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp:4197
+ AAMDNodes(), nullptr, SyncScope::System, AtomicOrdering::Unordered);
+
+ auto MIBAtomicRMWNand = B.buildAtomicRMWNand(OldValRes, Addr, Val, *MMO);
----------------
niwinanto wrote:
> 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.
MachineMemOperand is not a value. I'm talking about Addr and Val, which are undefined registers here
================
Comment at: llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp:4198
+
+ auto MIBAtomicRMWNand = B.buildAtomicRMWNand(OldValRes, Addr, Val, *MMO);
+
----------------
niwinanto wrote:
> arsenm wrote:
> > can just specify the return type here
> You meant `auto MIBAtomicRMWNand` to `MachineInstrBuilder MIBAtomicRMWNand`, or?
auto Nand = B.buildAtomicRMW(LLT::scalar(32))
================
Comment at: llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp:4211
+ // Check
+ EXPECT_TRUE(CheckMachineFunction(*MF, CheckStr)) << *MF;
+}
----------------
niwinanto wrote:
> arsenm wrote:
> > This should really fail with use of undefined value
> I don't really get this. What do you mean?
You didn't define the input registers for the instruction
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D152538/new/
https://reviews.llvm.org/D152538
More information about the llvm-commits
mailing list