patch: remove PseudoSourceValue from Value.

[Chris Lattner]

On Mar 4, 2014, at 9:12 PM, Nick Lewycky <nlewycky at google.com> wrote:

> On 4 March 2014 13:59, Evan Cheng <evan.cheng at apple.com> wrote:
> Sorry, I need to have better mail filtering.
> 
> PseudoSourceValue is a bad hack that's long on my kill list. Thank you for tackling this. Would you consider going further than your current patch and kill it completely?
> 
> I'm trying to get some undefined behaviour removed from LLVM, and I'm pulling the thread of llvm::Value having a vtable. This would be a diversion ...
> 
> ... but I realize that doing this patch now and then removing PSV later would break out of tree targets twice, and it would be nice to avoid that. That's a good reason to do it all at once.
> 
> The idea is to enhance MachinePointerInfo so the value can be an IR pointer value (i.e. const Value *V) or some immediate that represents special type such as constant pool, stack objects, etc. As a bonus, we would also want a mechanism to represent target specific tag to model special dependencies (common in non-CPU world). Your change in MachineMemOperand.h seems half way there.
> 
> Could you expand on this? How would that immediate be different from PseudoSourceValue after this change? Would we have the four PSVs (stack, GOT, JumpTable, ConstantPool) plus a Value PSV that contains a Value*? And targets could add their own to the list of 4 I mentioned?
> 
> Basically, I don't know enough to fill in the details of design you're describing, but I'm willing to do the actual coding/testing/debugging bits.

My personal problem with PseudoSourceValue is that is perverts the Value hierarchy with things that are not IR values... originally so that the AA interfaces could take machine "pointers" and somehow do something sensible with them.

This is a flawed design.  The only way this could work is if the "MachineAA" level interface successfully catches any (e.g.) stack address entries and avoids passing them do into the IR-level AA interfaces.

Given that, we're getting no benefit from mashing them in as Value*'s and we're also getting no help from the C++ type system.  A better way to go would be to make PseudoSourceValue be a pointer-sized discriminated union that is *either* a Value*, a frame index, constant pool index, etc.  This would require clients to unwrap all the special cases (verifying they actually have a value*) before calling the IR level interfaces, and it would give us a place to put various query APIs (on PseudoSourceValue itself).

While we're at it, it should be renamed to something that starts with "Machine". :-)

-Chris



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140305/e4e34dfc/attachment.html>