This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: V4 [PATCH 2/4] i386: Align branches within a fixed boundary
On Thu, Dec 12, 2019 at 8:46 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 12.12.2019 17:22, H.J. Lu wrote:
> > On Thu, Dec 12, 2019 at 8:10 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 12.12.2019 16:54, H.J. Lu wrote:
> >>> On Thu, Dec 12, 2019 at 1:55 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>
> >>>> On 11.12.2019 17:58, H.J. Lu wrote:
> >>>>> On Wed, Dec 11, 2019 at 12:05 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>>>
> >>>>>> On 10.12.2019 17:21, H.J. Lu wrote:
> >>>>>>> @@ -8473,9 +8815,105 @@ output_insn (void)
> >>>>>>> if (j > 15)
> >>>>>>> as_warn (_("instruction length of %u bytes exceeds the limit of 15"),
> >>>>>>> j);
> >>>>>>> + else if (fragP)
> >>>>>>> + {
> >>>>>>> + /* NB: Don't add prefix with GOTPC relocation since
> >>>>>>> + output_disp() above depends on the fixed encoding
> >>>>>>> + length. Can't add prefix with TLS relocation since
> >>>>>>> + it breaks TLS linker optimization. */
> >>>>>>> + unsigned int max = i.has_gotpc_tls_reloc ? 0 : 15 - j;
> >>>>>>> + /* Prefix count on the current instruction. */
> >>>>>>> + unsigned int count = i.vex.length;
> >>>>>>> + unsigned int k;
> >>>>>>> + for (k = 0; k < ARRAY_SIZE (i.prefix); k++)
> >>>>>>> + /* REX byte is encoded in VEX/EVEX prefix. */
> >>>>>>> + if (i.prefix[k] && (k != REX_PREFIX || !i.vex.length))
> >>>>>>> + count++;
> >>>>>>> +
> >>>>>>> + /* Count prefixes for extended opcode maps. */
> >>>>>>> + if (!i.vex.length)
> >>>>>>> + switch (i.tm.opcode_length)
> >>>>>>> + {
> >>>>>>> + case 3:
> >>>>>>> + if (((i.tm.base_opcode >> 16) & 0xff) == 0xf)
> >>>>>>> + {
> >>>>>>> + count++;
> >>>>>>> + switch ((i.tm.base_opcode >> 8) & 0xff)
> >>>>>>> + {
> >>>>>>> + case 0x38:
> >>>>>>> + case 0x3a:
> >>>>>>> + count++;
> >>>>>>> + break;
> >>>>>>> + default:
> >>>>>>> + break;
> >>>>>>> + }
> >>>>>>> + }
> >>>>>>> + break;
> >>>>>>> + case 2:
> >>>>>>> + if (((i.tm.base_opcode >> 8) & 0xff) == 0xf)
> >>>>>>> + count++;
> >>>>>>> + break;
> >>>>>>> + case 1:
> >>>>>>> + break;
> >>>>>>> + default:
> >>>>>>> + abort ();
> >>>>>>> + }
> >>>>>>> +
> >>>>>>> + if (TYPE_FROM_RELAX_STATE (fragP->fr_subtype)
> >>>>>>> + == BRANCH_PREFIX)
> >>>>>>> + {
> >>>>>>> + /* Set the maximum prefix size in BRANCH_PREFIX
> >>>>>>> + frag. */
> >>>>>>> + if (fragP->tc_frag_data.max_bytes > max)
> >>>>>>> + fragP->tc_frag_data.max_bytes = max;
> >>>>>>> + if (fragP->tc_frag_data.max_bytes > count)
> >>>>>>> + fragP->tc_frag_data.max_bytes -= count;
> >>>>>>> + else
> >>>>>>> + fragP->tc_frag_data.max_bytes = 0;
> >>>>>>> + }
> >>>>>>> + else
> >>>>>>> + {
> >>>>>>> + /* Remember the maximum prefix size in FUSED_JCC_PADDING
> >>>>>>> + frag. */
> >>>>>>> + unsigned int max_prefix_size;
> >>>>>>> + if (align_branch_prefix_size > max)
> >>>>>>> + max_prefix_size = max;
> >>>>>>> + else
> >>>>>>> + max_prefix_size = align_branch_prefix_size;
> >>>>>>> + if (max_prefix_size > count)
> >>>>>>> + fragP->tc_frag_data.max_prefix_length
> >>>>>>> + = max_prefix_size - count;
> >>>>>>> + }
> >>>>>>> +
> >>>>>>> + /* Use existing segment prefix if possible. Use CS
> >>>>>>> + segment prefix in 64-bit mode. In 32-bit mode, use SS
> >>>>>>> + segment prefix with ESP/EBP base register and use DS
> >>>>>>> + segment prefix without ESP/EBP base register. */
> >>>>>>> + if (i.prefix[SEG_PREFIX])
> >>>>>>> + fragP->tc_frag_data.default_prefix = i.prefix[SEG_PREFIX];
> >>>>>>> + else if (flag_code == CODE_64BIT)
> >>>>>>> + fragP->tc_frag_data.default_prefix = CS_PREFIX_OPCODE;
> >>>>>>> + else if (i.base_reg
> >>>>>>> + && (i.base_reg->reg_num == 4
> >>>>>>> + || i.base_reg->reg_num == 5))
> >>>>>>> + fragP->tc_frag_data.default_prefix = SS_PREFIX_OPCODE;
> >>>>>>> + else
> >>>>>>> + fragP->tc_frag_data.default_prefix = DS_PREFIX_OPCODE;
> >>>>>>> + }
> >>>>>>
> >>>>>> Could you point me at where it is being excluded that any of
> >>>>>> these prefixes would get added to insns where they would have
> >>>>>> the meaning of a hint (taken / not taken or no-track)?
> >>>>>
> >>>>> taken / not taken or no-track prefixes are for branches. We never
> >>>>> add prefixes to branches.
> >>>>
> >>>> But this was my question - could you point me at the code making
> >>>> sure this won't happen? I in particular can't spot anything to this
> >>>> effect in add_branch_prefix_frag_p().
> >>>>
> >>>
> >>> There are
> >>>
> >>> if (branch)
> >>> /* Skip if this is a branch. */
> >>> ;
> >>> else if (add_fused_jcc_padding_frag_p ())
> >>> ...
> >>> else if (add_branch_prefix_frag_p ())
> >>>
> >>> add_branch_prefix_frag_p is never called on branch nor fused jcc.
> >>
> >> Ah, I see. Of course add_branch_padding_frag_p() does neither deal
> >> with all kinds of branches, nor does it set the caller's branch
> >> variable to other than align_branch_none. This may be only a
> >> latent issue, but I assume you're aware of the SDM stating "use
> >> with any branch instruction is reserved" for every single segment
> >> override prefix (which, possibly wrongly, includes far branches as
> >> well as JCXZ/LOOP et al).
> >>
> >
> > There are
> >
> > if (add_branch_padding_frag_p (&branch))
> > {
> > }
> >
> > /* Output jumps. */
> > if (i.tm.opcode_modifier.jump == JUMP)
> > output_branch ();
> > else if (i.tm.opcode_modifier.jump == JUMP_BYTE
> > || i.tm.opcode_modifier.jump == JUMP_DWORD)
> > output_jump ();
> > else if (i.tm.opcode_modifier.jump == JUMP_INTERSEGMENT)
> > output_interseg_jump ();
> > else
> > {
> >
> > We don't add prefix to any branches.
>
> Oh, okay - sorry for not noticing, and thanks for clarifying.
>
I am going to check it in later today. We may update it in the future
based on feedbacks.
Thanks.
--
H.J.