patch: construct IntrinsicInst hierarchy directly

[Philip Reames]

On 02/18/2014 09:27 PM, Nick Lewycky wrote:
> LLVM's hand-rolled RTTI differs from C++'s in that it allows you to 
> call the construction method on a base class and then based on the 
> dynamic arguments decide to construct a more derived type and return a 
> pointer to that. This looks like calling CallInst::Create and getting 
> back something you can cast<MemSetInst>(). However, we implement this 
> by new'ing a CallInst, which breaks C++ aliasing rules.
>
> The attached patch fixes this undefined behaviour by making CallInst 
> forward to IntrinsicInst to construct intrinsics, which in turn uses 
> private constructors on the most derived types. There is some runtime 
> cost, as the implementation of CallInst::Create moved from the header 
> to the .cpp file. If this is a serious problem, let me know and I can 
> probably fix that, I just think the result will result in less much 
> readable code.
>
> Please review!
My only correctness concern would be the removal of:
void operator=(const IntrinsicInst&) LLVM_DELETED_FUNCTION;
from IntrinsicInst.  That looks suspicious.

I agree with David on the yuck factor of that much duplicated code 
though.  Templates, or other copy removal would be appreciated, but not 
required.

Another point: is there any way to detect if someone adds a new subclass 
of Instrinsic which needs added to your switch?  Maybe an assert in the 
protected constructor which is only satisfied coming from Create(...)?  
Seems a bit hacky, but might save someone a lot of debugging time.  
Again, appreciated, not required.

Otherwise, LGTM.

Philip