patch: remove PseudoSourceValue from Value.

[Nick Lewycky]

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 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
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140303/3c8a2340/attachment.html>