Re:Re: [PATCH v3 0/5] LoongArch tls le model linker relaxation support.

[常佳琛 <changjiachen@stu.xupt.edu.cn>]

[-- Attachment #1: Type: text/plain, Size: 3012 bytes --]









From: Xi Ruoyao <xry111@xry111.site>
Date: 2023-12-15 20:34:50
To:  changjiachen <changjiachen@stu.xupt.edu.cn>,binutils@sourceware.org
Cc:  xuchenghua@loongson.cn,chenglulu@loongson.cn,liuzhensong@loongson.cn,i.swmail@xen0n.name,maskray@google.com,cailulu@loongson.cn,luweining@loongson.cn,wanglei@loongson.cn,hejinyang@loongson.cn,Lazy_Linux@126.com,mengqinggang@loongson.cn
Subject: Re: [PATCH v3 0/5] LoongArch tls le model linker relaxation support.>On Fri, 2023-12-15 at 18:06 +0800, changjiachen wrote:
>> First, after a test, the R_LARCH_TLS_LE_ADD_R can be generated using ".reloc.,R_LARCH_TLS_LE_ADD_R,sym", 
>> or "%le_add_r(sym)". However,".reloc" generates R_LARCH_TLS_LE_ADD_R relocation directly, and it is not 
>> easy to add "R_LARCH_RELAX" relocation. "%le_add_r(sym)" Adds the R_LARCH_TLS_LE_ADD_R and R_LARCH_RELAX 
>> relocation commands, which is easier to implement.
>>
>> Of course, there is another way to generate ".reloc.,R_LARCH_TLS_LE_ADD_R,sym" and 
>> ".reloc.,R_LARCH_RELAX,sym" directly in gcc. However, this implementation causes the 
>> -mrelax/-mno-relax option to be set in both gcc and gas, which can be confusing.
>
>GCC 14 already have -mrelax/-mno-relax options.  This is not confusing
>at all.
>
>> One problem with this is that add.d $r12,$r12,$r2 and add.d $r12,$r12,$r2, 
>> %le_add_r(sym) are too similar, so I have to add comments in loongarch_fix_opcodes[]. 
>> The goal is to make it as clear as possible to developers.
>
>No, normal developers (not Binutils developers) should not be mandated
>to read Binutils code to understand something.  OTOH they should read
>the documentation of GAS.  So make it clear somewhere in the doc.
>
>And I've told you if you must go with an additional operand in add.d,
>you should reject things like:
>
>add.d $a0,$a0,$a0,8
>
>but now:
>
>$ cat t.s
>add.d $a0,$a0,$a0,8
>$ gas/as-new t.s
>$ binutils/objdump -d a.out
>
>a.out:     file format elf64-loongarch
>
>
>Disassembly of section .text:
>
>0000000000000000 <.text>:
>   0:	0010b084 	add.d       	$a0, $a0, $t0
>
>Now is it clear that this behavior is unacceptable?
>
>Is it too difficult or unreasonable to make it an error with message
>"the fourth operand of the add.d instruction must be %le_add_r"?!  Or
>didn't I make it clear?
Hello, I understand what you mean. For the fourth operand of tls add.d instruction, 
I have made the following modifications.


For the case of "add.d $a0,$a0,$a0,8", that is, when the fourth operand of the tls add.d 
instruction is not %le_add_r(sym), the modified assembler throws a "no match insn add.d $a0,$a0,$a0,8".


example：
a.s
add.d $a0,$a0,$a0,8
$ gas/as-new a.s
a.s: Assembler messages:
a.s:1: error：no match insn: add.d	$a0,$a0,$a0,8



Can such modification solve the problem you raised? 
I will add this change to v4 patch if possible.
I am very much looking forward to your suggestions again.








>
>-- 
>Xi Ruoyao <xry111@xry111.site>
>School of Aerospace Science and Technology, Xidian University