patch: remove PseudoSourceValue from Value.

Hal Finkel hfinkel at anl.gov
Mon Mar 3 22:07:48 PST 2014


----- Original Message -----
> From: "Nick Lewycky" <nlewycky at google.com>
> To: "Commit Messages and Patches for LLVM" <llvm-commits at cs.uiuc.edu>, "Eric Christopher" <echristo at google.com>
> Cc: "Hal Finkel" <hfinkel at anl.gov>, "Evan Cheng" <evan.cheng at apple.com>
> Sent: Monday, March 3, 2014 8:09:44 PM
> Subject: Re: patch: remove PseudoSourceValue from Value.
> 
> 
> 
> 1 week ping!
> 
> 
> +Eric Christopher, I'd also appreciate a review.
> 
> Hal, do you have any further review comments? (Does it look good to
> you, but you don't feel like you have sufficient knowledge to OK a
> commit?)

I have no further comments; I think that, technically, it looks fine. I also think that the design change itself makes sense, but would like at least someone else to concur.

 -Hal

> 
> 
> I think that between the three of us, Hal, Eric and myself, that
> would be sufficient eyes to approve the patch for commit.
> 
> 
> As a note to everybody else, this is likely to break out of tree
> targets when it lands. My current plan is to send an email to
> llvm-dev shortly before committing this patch, after I get the OK to
> commit.
> 
> 
> Nick
> 
> 
> 
> On 24 February 2014 17:19, Nick Lewycky < nlewycky at google.com >
> wrote:
> 
> 
> 
> +cc Evan Cheng
> 
> 
> Evan, I'm not sure whether you're still the right person to review
> this. I consider it a substantive change to how the backend works,
> even though the line count of the patch is small. If nothing else, I
> figure you want to be aware of this patch before it lands.
> 
> 
> 
> Nick
> 
> 
> 
> 
> On 24 February 2014 17:01, Nick Lewycky < nlewycky at google.com >
> wrote:
> 
> 
> 
> 
> 
> 
> 
> On 24 February 2014 16:45, Hal Finkel < hfinkel at anl.gov > wrote:
> 
> 
> 
> ----- 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?
> 
> 
> 
> Done.
> 
> 
> 
> - 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)?
> 
> 
> 
> Done. Updated patch attached.
> 
> 
> Nick
> 
> 
> 
> 
> 

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



More information about the llvm-commits mailing list