[PATCH] x86: imply all No_*Suf when none is set in a template

Jan Beulich jbeulich@suse.com
Fri Nov 22 13:43:00 GMT 2019


On 22.11.2019 08:35,  H.J. Lu  wrote:
> On Mon, Nov 18, 2019 at 5:10 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 15.11.2019 19:05,  H.J. Lu  wrote:
>>> On Fri, Nov 15, 2019 at 12:35 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>
>>>> On 14.11.2019 20:26, H.J. Lu wrote:
>>>>> On Thu, Nov 14, 2019 at 12:51 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>>
>>>>>> Since no template would ever allow for none of them to be set, reduce
>>>>>> table size and improve readability and hence maintainability by implying
>>>>>> all of them to be set when a template specifies none.
>>>>>>
>>>>>> opcodes/
>>>>>> 2019-11-XX  Jan Beulich  <jbeulich@suse.com>
>>>>>>
>>>>>>         * i386-gen.c (process_i386_opcode_modifier): Widen scope of
>>>>>>         bwlq_suf. New local variable other_suf. Emit warning when all
>>>>>>         No_*Suf are set. Set all No_*Suf when none are set.
>>>>>>         * i386-opc.tbl: Drop No_bSuf, No_wSuf, No_lSuf, No_sSuf,
>>>>>>         No_qSuf, and No_ldSuf when they're all specified at the same
>>>>>>         time.
>>>>>
>>>>> I don't think it may the table more readable.   Why not simply add
>>>>>
>>>>> #define  No_Suf  No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf
>>>>>
>>>>> and use it?
>>>>
>>>> Because this would require yet another global change once we switch
>>>> to positive identification of the permitted suffixes (which should
>>>> have been done from the beginning imo). Since the set of suffixes
>>>> that can go together is pretty limited (i.e. a fair subset of the
>>>> combinations currently possible to specify do never occur), my plan
>>>> is to switch to an enumeration here, containing enumerators like
>>>> bwlqSuf or wlqSuf.
>>>
>>> Replace or remove No_Suf should be very easy.
>>
>> "easy" is not a category of my thinking here. It's the huge amount
>> of code churn and the resulting unless-ness of e.g. "git blame" that
>> I'd like to avoid. Dropping the code again that this patch adds to
>> i386-gen.c is also "easy" enough.
> 
> I don't think "git blame" is an issue here.  No_Suf makes clear
> what exactly happens.

Well, I've put the patch onto my "rejected" list, as I'm not going
to put my name under something that causes this massive amount of
unnecessary code churn. I may reactivate it once I find time to do
the full conversion that I had outlined.

Jan



More information about the Binutils mailing list