On 24.09.2012, at 14:16, Paul Mackerras wrote: > On Fri, Sep 21, 2012 at 11:15:51AM +0200, Alexander Graf wrote: > >> So how about something like >> >> #define kvmppc_set_reg(id, val, reg) { \ >> switch (one_reg_size(id)) { \ >> case 4: val.wval = reg; break; \ >> case 8: val.dval = reg; break; \ >> default: BUG(); \ >> } \ >> } >> >> case KVM_REG_PPC_DAR: >> kvmppc_set_reg(reg->id, &val, vcpu->arch.shared->dar); >> break; > > I tried that and it was fine for the wval vs. dval thing, and gcc even > compiled it to the same number of instructions as what I had before. > But it breaks down when you get to VMX and VSX -- firstly because > there are two different types that are both 16 bytes, and secondly > because when you do this: > > #define kvmppc_get_reg(id, val, reg) { \ > switch (one_reg_size(id)) { \ > case 4: val.wval = reg; break; \ > case 8: val.dval = reg; break; \ > case 16: val.vval = reg; break; \ > default: BUG(); \ > } \ > } > > you get compile errors on things like: > > kvmppc_set_reg(reg->id, val, vcpu->arch.shared->dar); > > because val.vval is a vector128, and vcpu->arch.shared->dar is a u64, > and you can't assign a u64 to a vector128 since vector128 is a struct. > In other words, all of the cases of the switch have to satisfy the > type rules even though only one of them will ever be used. Basically > that trick will only work for integer types, and we don't have 128 bit > integer support in gcc. What a shame. Could you please repost a version that only handles 32/64 setting with the above helper then and leaves 128-bit the way they are implemented now? > Given all that, I would like to see my patches go in while we continue > to search for a way to avoid the potential mistakes you're talking > about. ... that way we can at least make the "common case" of 32-bit and 64-bit registers error proof and only have to worry more about double-checking the 128-bit ones, which are a lot less. Alex -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html