On 7/3/19 11:37 AM, Richard Henderson wrote: > On 7/1/19 6:35 AM, Jan Bobek wrote: >> + VEX_V_UNUSED => 0b1111, > > I think perhaps this is a mistake. Yes, that's what goes in the field, but > what goes in the field is ~(logical_value). > > While for general RISU-ish operation, ~(random_number) is just as random as > +(random_number), the difference will be if we ever want to explicitly emit > with this interface a specific vex instruction which also requires the > v-register.
See below. >> +sub rex_encode(%) >> +{ >> + my (%args) = @_; >> + >> + $args{w} = 0 unless defined $args{w}; >> + $args{r} = 0 unless defined $args{r}; >> + $args{x} = 0 unless defined $args{x}; >> + $args{b} = 0 unless defined $args{b}; >> + >> + return (value => 0x40 >> + | (($args{w} ? 1 : 0) << 3) >> + | (($args{r} ? 1 : 0) << 2) >> + | (($args{x} ? 1 : 0) << 1) >> + | ($args{b} ? 1 : 0), >> + len => 1); >> +} > > Does > > (defined $args{w} && $args{w}) << 3 > > work? That seems tidier to me than splitting these conditions. It does, I will change it. Thanks! >> + return (value => (0xC4 << 16) >> + | (($args{r} ? 1 : 0) << 15) >> + | (($args{x} ? 1 : 0) << 14) >> + | (($args{b} ? 1 : 0) << 13) > > Further down in vex_encode, and along the lines of VEX_V_UNUSED, this appears > to be actively wrong, since these bits are encoded as inverses. What this > *really* means is that because of that, rex_encode and vex_encode will not > encode the same registers for a given instruction. Which really does feel > bug-like, random inputs or no. So, vex_encode, rex_encode and friends were meant to be really low-level functions; they literally just encode the bits from what you pass in, without any concern for what the fields even mean. In that spirit, write_insn itself never did much of error-checking. I have added quite a lot of code to risugen_x86_asm in v3; most importantly, there are now asm_insn_* functions which are more high-level, in that you pass in the logical values and they care of error checks and encoding. I also removed write_insn and all the encoding-related symbolic constants from the public interface of the module. -Jan
signature.asc
Description: OpenPGP digital signature