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: V4 [PATCH 2/4] i386: Align branches within a fixed boundary


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.

Jan


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