[PATCH] RC->contains is a trap... (PR16106)

Eric Christopher echristo at gmail.com
Wed Jun 12 16:36:02 PDT 2013


>
> OK, here we go:
>
> I don’t really like to think about the allocation order as defining the set of registers that can be used legally. I see it more as a hint to the register allocator about the preferred registers. Removing a register from the allocation order is a very strong hint to avoid it, but it shouldn’t be illegal.
>

For what it's worth, I agree mostly. Until you hit the part where you
said if it was removed it was still legal... more on that in a second.

> In most cases, registers that are not supported by a particular subtarget can simply be reserved, like we do for r8-r15 and xmm8-xmm15.
>
> TableGen is actually using the defined register classes to synthesize more register classes so that you get a closed algebra under intersection and sub-register operations. The idea is to provide enough register classes that MachineRegisterInfo::recomputeRegClass() can compute an exact answer.
>

Sure, I can understand the desire here.

> If the set of legal registers is changed by the allocation order, TableGen isn’t using the right sets for its computations.
>
> This constraint isn’t really causing any problems that I am aware of, except for the x86 8-bit registers. Even there, the problems only arise because the 32 and 64-bit subtargets have completely different encoding constraints.
>

As far as I know x86 is the only target that uses AltOrder to change
the number of registers in a class.

> On x86-64, we avoid using the ‘H’ 8-bit registers since they cannot be encoded in an instruction with a REX prefix. We don’t reserve them, though, since they are still useful in some cases. They are only ever used with the *_NOREX pseudo-instructions.
>

Yes, agreed. There's also a specific register class for NOREX as well.

> I think the problem comes from using the GR8 register class directly. It has too many registers both for 32-bit and 64-bit mode. I wonder if it would help to define a GR8_L register class containing only the low 8-bit registers (no sub_8bit_hi regs), and then use that as the legal register class for i8 on x86-64.
>

OK, so that was the d) I wasn't bringing up.

I'm fine with someone (possibly even me) making this change, but
perhaps now we've decided to change the documented behavior of
AltOrders? :) I'm definitely not against it since, to me, the
definition of an AltOrder is an alternate ordering, not an entirely
different register class. Probably should add an assert in tablegen to
ensure that the N orderings all have the same size.

I'd rather make this change than have "well, the register is still
there, but you probably shouldn't use it" since that will cause subtle
bugs if someone makes a typo. Better to assert and not allow the
behavior if we don't want people to use it.

We can make the register class change for X86 - it's rather a large
change across the port (fast-isel in particular), but splitting
register classes involves churn :)

-eric




More information about the llvm-commits mailing list