Re: [Qemu-devel] [RISU PATCH v3 04/18] risugen_x86_constraints: add module

[Richard Henderson]

On 7/12/19 12:32 AM, Jan Bobek wrote:
> +sub vex($%)
> +{
> +    my ($insn, %vex) = @_;
> +    my $regidw = $is_x86_64 ? 4 : 3;
> +
> +    # There is no point in randomizing other VEX fields, since
> +    # VEX.R/.X/.B are encoded automatically by risugen_x86_asm, and
> +    # VEX.M/.P are opcodes.
> +    $vex{l} = randint(width => 1) ? 256 : 128 unless defined $vex{l};

VEX.L is sort-of opcode-like as well.  It certainly differentiates AVX1 vs
AVX2, and so probably should be constrained somehow.  I can't think of what's
the best way to do that at the moment, since our existing --xstate=foo isn't 
right.

Perhaps just a FIXME comment for now?

> +sub modrm_($%)
> +{
> +    my ($insn, %args) = @_;
> +    my $regidw = $is_x86_64 ? 4 : 3;
> +
> +    my %modrm = ();
> +    if (defined $args{reg}) {
> +        # This makes the config file syntax a bit more accommodating
> +        # in cases where MODRM.REG is an opcode extension field.
> +        $modrm{reg} = $args{reg};
> +    } else {
> +        $modrm{reg} = randint(width => $regidw);
> +    }
> +
> +    # There is also a displacement-only form, but we don't know
> +    # absolute address of the memblock, so we cannot test it.

32-bit mode has displacement-only, aka absolute; 64-bit replaces that with
rip-relative.  But agreed that the first is impossible to test and the second
is difficult.

> +sub modrm($%)
> +{
> +    my ($insn, %args) = @_;
> +    modrm_($insn, indexk => 'index', %args);
> +}

How are you avoiding %rsp as index?
I saw you die for that in the previous patch...


r~