This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH v3] Work around the NOP issue of Loongson2F
Wu Zhangjin <wuzhangjin@gmail.com> writes:
> [...]
>> >
>> > Even if the answer's no, I'm uncomfortable with one kind of nop being
>> > treated differently.
>>
>> The same to me, I will try to find out how does the .align and .fill
>> works and why it uses the 0 as the nop but doesn't use the NOP_INSN
>> initialized in md_begin(). what about your suggestion here? I have tried
>> to return NOP_INSN->insn_opcode in mips_nop_opcode() and modify
>> mips_handle_align():
>>
>> - md_number_to_chars (p, mips16_nop_insn.insn_opcode, 2);
>> + md_number_to_chars (p, NOP_INSN->insn_opcode, 2);
>>
>> It did generate the source code like this: 8250825,
>
> If do the following change:
>
> - fragp->fr_var = 2;
> + fragp->fr_var = 4;
>
> It works, so, the final solution:
>
> gas/config/tc-mips.c:
>
> @@ -14779,11 +14801,18 @@ static procS *cur_proc_ptr;
> static int numprocs;
>
> /* Implement NOP_OPCODE. We encode a MIPS16 nop as "1" and a normal
> - nop as "0". */
> + nop as "0".
> +
> + If -mfix-loongson2f is passed, we encode the nop as "1". This is
> + needed to enable the work around in mips_handle_align() for
> + loongson2f.
> + */
>
> char
>
> char
> mips_nop_opcode (void)
> {
> + if (mips_fix_loongson2f)
> + return 1;
> return seg_info (now_seg)->tc_segment_info_data.mips16;
> }
>
> @@ -14809,8 +14838,13 @@ mips_handle_align (fragS *fragp)
> *p++ = 0;
> fragp->fr_fix++;
> }
> - md_number_to_chars (p, mips16_nop_insn.insn_opcode, 2);
> - fragp->fr_var = 2;
> + if (mips_fix_loongson2f && !mips_opts.mips16) {
> + md_number_to_chars (p, nop_insn.insn_opcode, 4);
> + fragp->fr_var = 4;
> + } else {
> + md_number_to_chars (p, mips16_nop_insn.insn_opcode, 2);
> + fragp->fr_var = 2;
> + }
> }
> }
This isn't right:
- You're changing the behaviour for MIPS16 nops when -mfix-loongson2f
is specified. (This is a valid combination; the application might
only execute the MIPS16 code when MIPS16 support is detected.)
- "p" points to 3 bytes only, so you could get buffer overrun in the
Loongson nop.
- You're not handling the case where (bytes & 3) is either 2 or 3.
You need to add 2 or 3 zeros to the end of the fixed part of the
frag in that case, whereas the patch would add 0 and 1 respectively.
The right fix IMO is to use something like the following for
mips_handle_align:
char *p;
valueT opcode, size, bytes, excess;
if (fragp->fr_type != rs_align_code)
return;
/* Pick the opcode recorded by mips_nop_opcde. */
p = fragp->fr_literal + fragp->fr_fix;
if (*p)
{
opcode = mips16_nop_insn.insn_opcode;
size = 2;
}
else
{
opcode = nop_insn.insn_opcode;
size = 4;
}
bytes = fragp->fr_next->fr_address - fragp->fr_address - fragp->fr_fix;
excess = bytes % size;
if (excess != 0)
{
/* If we're not inserting a whole number of instructions, pad the
end of the fixed part of the frag with zeros. */
memset (p, 0, excess);
p += excess;
fragp->fr_fix += excess;
}
md_number_to_chars (p, opcode, size);
fragp->fr_var = size;
and then change the tc-mips.h definition of MAX_MEM_FOR_RS_ALIGN_CODE to:
#define MAX_MEM_FOR_RS_ALIGN_CODE (3 + 4)
(completely untested). No changes should be needed to mips_nop_opcode.
Richard