[PATCH] D102745: [OpaquePtr] Make cmpxchg work with opaque pointers

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/lib/IR/Verifier.cpp:3854-3856
   Assert(ElTy == CXI.getOperand(2)->getType(),
-         "Stored value type does not match pointer operand type!", &CXI, ElTy);
+         "Stored value type does not match expected value operand type!", &CXI,
+         ElTy);
----------------
aeubanks wrote:
> dblaikie wrote:
> > Is there missing testing for this? Looks like the wording/assert was changed, but no test cases were updated for this?
> I think we could ever reasonably reach these since the bitcode reader and llparser will both already give up if this were to fail. And there's a similar assert when actually creating the instructions.
> So I'm not sure how to actually test this. Maybe we should just remove these?
Yeah, if they're unreachable I'd be inclined to remove them.

The only thing that comes to mind would be in a non-asserts build, using a unit test/API testing - build it then verify. But that's testing behavior that's out-of-contract (behavior behind a failed assertion) anyway, which isn't reasonable in my opinion.

So, yeah, if you can't get there in a +Asserts build via API, bitcode, or textual IR, I'd say it's dead code & be fine with dropping it (probably do that in a separate preliminary commit, though)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102745