This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] aarch64: actually copy first operand in convert_bfc_to_bfm()


On 21/02/17 10:38, Jan Beulich wrote:
>>>> On 21.02.17 at 11:30, <Richard.Earnshaw@arm.com> wrote:
>> On 21/02/17 10:11, Jan Beulich wrote:
>>> Commit 93562a343c ("[AArch64] PR target/20666, fix wrong encoding of
>>> new introduced BFC pseudo") changed the destination operand to 0,
>>> making the whole function invocation a no-op. We really want to copy
>>> operand 0 (a register) to operand 1 (an immediate before coming here).
>>>
>>> opcodes/
>>> 2017-02-21  Jan Beulich  <jbeulich@suse.com>
>>>
>>> 	* aarch64-asm.c (convert_bfc_to_bfm): Copy operand 0 to operand
>>> 	1 (instead of to itself).
>>>
>>> --- 2017-02-21/opcodes/aarch64-asm.c
>>> +++ 2017-02-21/opcodes/aarch64-asm.c
>>> @@ -1607,7 +1607,7 @@ convert_bfc_to_bfm (aarch64_inst *inst)
>>>    /* Insert XZR.  */
>>>    copy_operand_info (inst, 3, 2);
>>>    copy_operand_info (inst, 2, 1);
>>> -  copy_operand_info (inst, 0, 0);
>>> +  copy_operand_info (inst, 1, 0);
>>>    inst->operands[1].reg.regno = 0x1f;
>>>  
>>>    /* Convert the immedate operand.  */

Just spotted a typo: immediate

>>
>> This needs an update to the testsuite as well.  Clearly something is
>> missing there or it would have been found before.
> 
> I don't understand: At present the difference between the operand
> being an immediate or a register simply doesn't matter (anymore)
> once this function ran. 

It wasn't obvious from your post that this was the case and a casual
glance at it wasn't immediately enlightening without the additional context.

> There's no resulting change in generated
> code; this is just a latent bug that I spotted since my fix to the
> original problem differed from the one which got committed (and I
> did point this out back then).
> 
> Jan
> 

Actually, there might be.  Immediate operands might still have 'skip'
set on them if convert_mov_to_movebitmask is anything to go by (whether
you can get to this state through this pattern is another thing though).

OK, but can you also fix the typo while committing.

R.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]