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

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks 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);
----------------
dblaikie wrote:
> 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)
sounds good, I'll delete these in a future change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102745