Page MenuHomePhabricator

[Verifier] Rephrase visitIntrinsicFunctionCall in terms of CallSites
ClosedPublic

Authored by reames on May 28 2015, 10:14 PM.

Details

Summary

The motivation is to remove a special case for invokable statepoints and share the code for intrinsic verification for CallInst and InvokeInst.

The code change is intentionally very mechanical. I left off variable renaming - which will happen in a follow on change - to reduce the textual diff to simplify review. The only part that really needs review is a) whether adding all the methods to CallSite is reasonable to make it closer to a drop in replacement for CallInst, and b) whether extending Write to support CallSites directly is reasonable. (I used the CallSite * to reduce the textual diff for converting a function from CallInst to CallSite. I'm open to changing this, but think the current version is worthwhile, even if ugly.)

Diff Detail

Repository
rL LLVM

Event Timeline

reames updated this revision to Diff 26768.May 28 2015, 10:14 PM
reames retitled this revision from to [Verifier] Rephrase visitIntrinsicFunctionCall in terms of CallSites.
reames updated this object.
reames edited the test plan for this revision. (Show Details)
reames added a subscriber: Unknown Object (MLST).
sanjoy accepted this revision.Jun 17 2015, 6:47 PM
sanjoy edited edge metadata.

I think this is okay to check in, modulo three inline comments (and that the s/CI/CS fix happens post-commit). However, it would be nice if someone else who's touched CallSite.h also takes a look.

include/llvm/IR/CallSite.h
84 ↗(On Diff #26768)

I think the type should be a template parameter so that we return a const BasicBlock * from an ImmutableCallSite and a BasicBlock * from a normal CallSite.

200 ↗(On Diff #26768)

Should be ValTy *.

lib/IR/Verifier.cpp
108 ↗(On Diff #26768)

Why not just Assert(..., CS.getInstruction()) instead?

This revision is now accepted and ready to land.Jun 17 2015, 6:47 PM
This revision was automatically updated to reflect the committed changes.