[llvm-commits] [PATCH] Execution domain support for VMOV and VLDR

[Jakob Stoklund Olesen]

On Aug 28, 2012, at 10:42 AM, Tim Northover <t.p.northover at gmail.com> wrote:

> And if we don't have to track kill/dead then that solution becomes
> more workable. Which leaves us with three options along those lines:
> 1. A loop before the switch, which stores the removed operands for
> later use (they're unknown in number and form so presumably in a
> SmallVector of operands).
> 2. A loop in each case, after saving the bits we need.
> 3. Unrolled in each case, since we know how many operands there are at
> that point. This is identical to the original patches as far as I can
> see.
> 
> I'm unconvinced of the benefit of 2 over 3 (though laziness may be a
> factor there).

I would prefer 2/3 over 1 because it adds readability. This:

      MIB.addReg(DReg, RegState::Define)
         .addReg(DReg, RegState::Undef)
         .addReg(SrcReg)
         .addImm(Lane);

Is much better than:

      MIB.addReg(Saved[0].getReg(), RegState::Define)
         .addReg(Saved[0].getReg(), RegState::Undef)
         .addOperand(Saved[1])
         .addImm(Lane);

It's quite confusing when you mix parsing of the old instruction with building the new one.

As for 2 vs 3, do what you like. The loop is safer to copy/paste.

I think it would also help if you add comments with examples of the instructions you're parsing and building.

Thanks,

/jakob