[PATCH] D16530: [Polly] Introduce MemAccInst helper class; NFC

[Michael Kruse via llvm-commits]

Meinersbur added a comment.

In http://reviews.llvm.org/D16530#335899, @grosser wrote:

> Hi Michael,
>
> I think this patch makes a lot of sense. Unfortunately all these casts make it look rather ugly as they introduce more noise than is actually removed. I just looked quickly at the implementation of CallSite and it seems the implementation is slightly different. Specifically, it does not inherit from instruction, but rather creates a proxy class around the CallInst/InvokeInst. Like this it is possible to add default constructors for CallSite(Invoke Ty) and CallSite(CallTy) that allow for automatic implicit type conversion. Could this be a solution that would allow us to remove the need for explicit casts?


Unfortunately their type is LoadInst* instead of LoadInst&. C++ does not support overloading operators/implicit conversions of pointer types. I could change their declarations to references and implement these implicit constructors, but this would create a new MemAccInst every time, i.e. create a new instruction because it is derived from Instruction.

The implementation of CallSite is quite different. It is a stack object containing a pointer to the CallInst or InvokeInst (you called it proxy). This allows "it can also create a null initialized CallSiteBase object for something which is NOT a call site". I don't think we need this, we can just test for nullptr. It also does not allow implicit conversions from pointers and users then are required to think about the lifetime of the stack object. It would allow implicit conversions from LoadInst& though.

Anyway, it is only one of the uses where I thought it could make sense. We could also just drop the BlockGenerator::generateLocationAccessed part.

One more use could be InstructionToAccess mapping, because since http://reviews.llvm.org/D13676 we only map loads/stores.


http://reviews.llvm.org/D16530