mir-canon simplified patch

[Puyan Lotfi via llvm-commits]

Alright, here's another revised patch.

PL



On Wed, Nov 1, 2017 at 2:51 AM, Justin Bogner <mail at justinbogner.com> wrote:

> Puyan Lotfi <puyan.lotfi.llvm at gmail.com> writes:
> > On Tue, Oct 31, 2017 at 2:15 AM, Justin Bogner <mail at justinbogner.com>
> wrote:
> >> Puyan Lotfi <puyan.lotfi.llvm at gmail.com> writes:
> >> > Thanks Justin!
> >> >
> >> > I have an updated patch, addressing the provided feedback.
> >>
> >> Looking better. I have a few more nitpicks below, and this could
> >> probably use a couple of simple tests for the basic functionality.
>  ...
> >> > +  TypedVReg(const TypedVReg &TR): type(TR.type), reg(TR.reg) { }
> >> > +
> >> > +  TypedVReg& operator=(const TypedVReg &rhs) {
> >> > +    type = rhs.type;
> >> > +    reg = rhs.reg;
> >> > +    return *this;
> >> > +  }
> >>
> >> These should both be default, so just omit them (or write them as =
> >> default if you're feeling verbose).
> >
> > I can be sure the default = operator is going to copy the members from
> rhs
> > to this?
>
> Yes, the default operator= does memberwise copy (or memmove-like
> semantics when the operator is trivial, like in this case). Leaving it
> as the default also means you'll get move assignment and such for free.
>
> There's a good summary of the standard here:
>
>   http://en.cppreference.com/w/cpp/language/copy_assignment
>
> >> > +  const TargetRegisterClass *DummyRC = [] (const MachineFunction
> &MF) {
> >> > +    const unsigned DummyVReg = GetDummyVReg(MF);
> >> > +    assert(DummyVReg != ~0U && "Could not find a dummy VReg.");
> >>
> >> So this pass doesn't work at all if we give it input with no vregs. That
> >> doesn't seem great - maybe we'd be better off making something up in
> >> that case.
> >
> > Maybe I should bail out of the vreg renaming portion but continue the
> > instruction ordering?
>
> That sounds reasonable to me as a first shot at this. We can make it
> more clever in follow ups.
>
> > I think I shouldn't assert but if I can't find a vreg then in the current
> > form of the pass im not sure exactly what I could rename in the first
> > place. Hmm..
> >
> > I'm still working on another patch revision. Will reply with it soon.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171101/674f7baf/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mir-canon-simplified3.patch
Type: text/x-diff
Size: 22506 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171101/674f7baf/attachment.patch>