patch: remove PseudoSourceValue from Value.

[Hal Finkel]

----- Original Message -----
> From: "Chris Lattner" <clattner at apple.com>
> To: "Nick Lewycky" <nlewycky at google.com>
> Cc: "Eric Christopher" <echristo at google.com>, "Commit Messages and Patches for LLVM" <llvm-commits at cs.uiuc.edu>
> Sent: Wednesday, March 5, 2014 12:37:00 PM
> Subject: Re: patch: remove PseudoSourceValue from Value.
> 
> 
> 
> 
> 
> 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.

I think that we will need a real MachineAA soon, but I see no reason that it should take Value*s at its interface.

 -Hal

> 
> 
> 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
> 
> 
> 
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory