mir-canon simplified patch

[Justin Bogner via llvm-commits]

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.