patch: remove PseudoSourceValue from Value.

[Hal Finkel]

----- Original Message -----
> From: "Nick Lewycky" <nlewycky at google.com>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: "Commit Messages and Patches for LLVM" <llvm-commits at cs.uiuc.edu>
> Sent: Monday, February 24, 2014 6:31:34 PM
> Subject: Re: patch: remove PseudoSourceValue from Value.
> 
> 
> 
> 
> On 24 February 2014 16:20, Hal Finkel < hfinkel at anl.gov > wrote:
> 
> 
> What is the underlying motivation for doing this?
> 
> 
> 
> The proximate cause is that I'm trying to remove the vtable from
> llvm::Value, and in order to do that I'm going to write dispatch
> functions that switch on the ValueID and call the appropriate
> function from the most-derived-type. If I do that, then we have
> lib/IR placing calls to lib/CodeGen, which is a layering violation.
> 
> 
> At a higher level, PSV does not belong there. It is not part of the
> IR. No IR passes use or understand it. It's not documented in the
> LangRef. Frankly, it's a hack -- and it's a great hack that and I
> understand its utility for lib/CodeGen -- but that doesn't change
> the fact that it's not part of llvm's IR. I've just never been
> motivated to do something about it until now.

Thanks for explaining!

   AliasAnalysis::AliasResult AAResult = AA->alias(
-  AliasAnalysis::Location(MMOa->getValue(), Overlapa,
-                          UseTBAA ? MMOa->getTBAAInfo() : 0),
-  AliasAnalysis::Location(MMOb->getValue(), Overlapb,
-                          UseTBAA ? MMOb->getTBAAInfo() : 0));
+      AliasAnalysis::Location(MMOa->getValue(), Overlapa,
+                              UseTBAA ? MMOa->getTBAAInfo() : 0),
+      AliasAnalysis::Location(MMOb->getValue(), Overlapb,
+                              UseTBAA ? MMOb->getTBAAInfo() : 0));

This is purely an indentation change, can you please commit this separately?

-        MapVector<const Value *, std::vector<SUnit *> >::iterator IE =
+        MapVector<PointerUnion<const Value *, const PseudoSourceValue *>,
+            std::vector<SUnit *> >::iterator IE =

These are starting to get really ugly. Can you please do something so that we don't have to use these really long local types in many places (like make a local typedef)?

Thanks again,
Hal

> 
> 
> Nick
> 
> 
> 
> 
> ----- Original Message -----
> > From: "Nick Lewycky" < nlewycky at google.com >
> > To: "Commit Messages and Patches for LLVM" <
> > llvm-commits at cs.uiuc.edu >
> > Sent: Monday, February 24, 2014 6:13:30 PM
> > Subject: patch: remove PseudoSourceValue from Value.
> > 
> > 
> > 
> > The attached patch removes PseudoSourceValue from the Value
> > hierarchy. No functionality change intended, but I had to make a
> > number of assumptions in order to make that claim. I assume nobody
> > ever assigned names to PSV's. I assume nobody has an AliasAnalysis
> > that is smart about PSV's (it's not in tree). That sort of thing.
> > 
> > 
> > I'm going to say up front, this patch isn't entirely pretty.
> > There's
> > some PointerUnions involved. Some code did get nicer, there were
> > users that had tests for Value* then if isa<PseudoSourceValue> do
> > something that would end in an unconditional return or continue,
> > then afterwards handle non-PSV Value's. These are now sensible
> > conditions for the two cases. Some code got worse. Target/Mips wins
> > the contest for actually having code that treated PSV's and Value's
> > through the same code paths, good job guys! Sorry that I'm doing
> > this to you.
> > 
> > 
> > At its core, this patch makes PseudoSourceValue the root of its own
> > hierarchy with just itself and FixedPseudoSourceValue in it, then
> > updated MachinePointerInfo to store a pointer-union of the PSV and
> > Value*. Various bits of code throughout lib/CodeGen and lib/Target
> > are updated to pass MPI's around instead of extracting the Value*
> > from an MPI, passing that, then rebuilding the MPI on the other
> > end.
> > Such code was silly anyways.
> > 
> > 
> > Please review. Before we get a low-level code review, I'm expecting
> > to be told that I've violated some design criteria of the backend
> > that I wasn't aware of. I mean, I moved a method from private to
> > public, so that's a pretty big sign. Please feel free to tell me
> > how
> > things ought to be layered.
> > 
> > 
> > Nick
> > 
> > 
> > _______________________________________________
> > 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
> 

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