This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [Patch, AArch64] fix the encoding of MOV immediate instructions in NDEBUG build
- From: Tristan Gingold <gingold at adacore dot com>
- To: Richard Earnshaw <rearnsha at arm dot com>
- Cc: Yufeng Zhang <Yufeng dot Zhang at arm dot com>, "binutils at sourceware dot org" <binutils at sourceware dot org>
- Date: Mon, 17 Sep 2012 16:55:29 +0200
- Subject: Re: [Patch, AArch64] fix the encoding of MOV immediate instructions in NDEBUG build
- References: <505714D1.80802@arm.com> <505730BA.3010203@arm.com>
On Sep 17, 2012, at 4:16 PM, Richard Earnshaw wrote:
> On 17/09/12 13:17, Yufeng Zhang wrote:
>> Hi,
>>
>> This patch fixes an inappropriate assertion in AArch64 opcodes which
>> causes malfunction of gas in assembling MOV immediate instructions if
>> the tools are built with -DNDEBUG.
>>
>> The patch passes the regression test.
>>
>> OK to commit?
>>
>> Thanks,
>> Yufeng
>>
>>
>> opcodes/
>>
>> 2012-09-17 Yufeng Zhang <yufeng.zhang@arm.com>
>>
>> * aarch64-asm.c (aarch64_ins_imm_half): Remove
>> ATTRIBUTE_UNUSED from
>> the parameter 'inst'.
>> (aarch64_ins_addr_simm): Add ATTRIBUTE_UNUSED to the parameter
>> 'inst'.
>> (convert_mov_to_movewide): Change to assert (0) when
>> aarch64_wide_constant_p returns FALSE.
>>
>>
>
> OK.
>
> Tristan, this needs to go on the 2.23 branch as well please.
Fine with me.
Do you take care of it, or do you prefer me to merge it ?
(In the later case, would you like sending the binutils-cvs url of the commit message ?)
Tristan.
>
> R.
>
>> wide-constant.patch
>>
>>
>> diff --git a/opcodes/aarch64-asm.c b/opcodes/aarch64-asm.c
>> index e10240a..006b075 100644
>> --- a/opcodes/aarch64-asm.c
>> +++ b/opcodes/aarch64-asm.c
>> @@ -336,8 +336,7 @@ aarch64_ins_imm (const aarch64_operand *self, const aarch64_opnd_info *info,
>> MOVZ <Wd>, #<imm16>{, LSL #<shift>}. */
>> const char *
>> aarch64_ins_imm_half (const aarch64_operand *self, const aarch64_opnd_info *info,
>> - aarch64_insn *code,
>> - const aarch64_inst *inst ATTRIBUTE_UNUSED)
>> + aarch64_insn *code, const aarch64_inst *inst)
>> {
>> /* imm16 */
>> aarch64_ins_imm (self, info, code, inst);
>> @@ -532,7 +531,8 @@ aarch64_ins_addr_regoff (const aarch64_operand *self ATTRIBUTE_UNUSED,
>> const char *
>> aarch64_ins_addr_simm (const aarch64_operand *self,
>> const aarch64_opnd_info *info,
>> - aarch64_insn *code, const aarch64_inst *inst)
>> + aarch64_insn *code,
>> + const aarch64_inst *inst ATTRIBUTE_UNUSED)
>> {
>> int imm;
>>
>> @@ -1090,8 +1090,9 @@ convert_mov_to_movewide (aarch64_inst *inst)
>> }
>> inst->operands[1].type = AARCH64_OPND_HALF;
>> is32 = inst->operands[0].qualifier == AARCH64_OPND_QLF_W;
>> - /* This should have been guaranteed by the constraint check. */
>> - assert (aarch64_wide_constant_p (value, is32, &shift_amount) == TRUE);
>> + if (! aarch64_wide_constant_p (value, is32, &shift_amount))
>> + /* The constraint check should have guaranteed this wouldn't happen. */
>> + assert (0);
>> value >>= shift_amount;
>> value &= 0xffff;
>> inst->operands[1].imm.value = value;
>>
>
>
>
>