[PATCH] x86: correct abort check
H.J. Lu
hjl.tools@gmail.com
Thu Dec 14 12:07:00 GMT 2017
On Wed, Dec 13, 2017 at 1:00 AM, Jan Beulich <JBeulich@suse.com> wrote:
> I'm rather certain the missing ! was just a typo, the more with the
> similar check in mind that's in the same function a few hundred lines
> down (in the body of "if (vex_reg != (unsigned int) ~0)"). Of course
> this can't be demonstrated by a test case - internal data structure
> consistency is being checked here, and neither form of the check
> triggers with any current template.
>
> It is also not really clear to me why operand_type_equal() is being used
> in the {X,Y,Z}MM register check here, rather than just testing the
> respective bits: Just like Reg32|Reg64 is legal in an operand template,
> I don't see why e.g. RegXMM|RegYMM wouldn't be. For example it ought to
> be possible to combine
>
> vaddpd, 3, 0x6658, None, 1, CpuAVX, Modrm|Vex|VexOpcode=0|VexVVVV=1|VexW=1|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Xmmword|Unspecified|BaseIndex|Disp8|Disp16|Disp32|Disp32S|RegXMM, RegXMM, RegXMM }
> vaddpd, 3, 0x6658, None, 1, CpuAVX, Modrm|Vex=2|VexOpcode=0|VexVVVV=1|VexW=1|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Ymmword|Unspecified|BaseIndex|Disp8|Disp16|Disp32|Disp32S|RegYMM, RegYMM, RegYMM }
>
> into a single template (with setting of VEX.L suitably handled elsewhere
> if that's not already happening anyway).
>
> Additionally I don't understand why this uses abort() instead of
> gas_assert().
I use abort for things which should never happen as sanity check.
> Both of these latter considerations then also apply to the
> aforementioned other check in the same function.
>
> gas/
> 2017-12-13 Jan Beulich <jbeulich@suse.com>
>
> * config/tc-i386.c (build_modrm_byte): Add missing ! to reg64
> check leading to abort().
>
> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -6437,7 +6437,7 @@ build_modrm_byte (void)
> if (i.tm.opcode_modifier.vexvvvv == VEXXDS)
> {
> /* For instructions with VexNDS, the register-only source
> - operand must be 32/64bit integer, XMM, YMM or ZMM
> + operand must be a 32/64bit integer, XMM, YMM, ZMM, or mask
> register. It is encoded in VEX prefix. We need to
> clear RegMem bit before calling operand_type_equal. */
>
> @@ -6459,7 +6459,7 @@ build_modrm_byte (void)
> op.bitfield.regmem = 0;
> if ((dest + 1) >= i.operands
> || (!op.bitfield.reg32
> - && op.bitfield.reg64
> + && !op.bitfield.reg64
> && !operand_type_equal (&op, ®xmm)
> && !operand_type_equal (&op, ®ymm)
> && !operand_type_equal (&op, ®zmm)
>
>
>
OK.
Thanks.
--
H.J.
More information about the Binutils
mailing list