This is the mail archive of the 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: [RFC PATCH] aarch64: ld: fix relaxations for ilp32 mode

On 17/01/17 17:04, Yury Norov wrote:
On Tue, Jan 17, 2017 at 12:20:39PM +0000, Jiong Wang wrote:
On 17/01/17 08:48, Yury Norov wrote:
On Mon, Jan 16, 2017 at 10:13:43AM +0000, Jiong Wang wrote:
On 14/01/17 10:48, Yury Norov wrote:
Hi all,

This patch continues the work of replacing 64-bit registers, offsets etc with
32-bit ones in elfNN_aarch64_tls_relax(). It doesn't fix any test I have tried,
but it's generally correct, and I think that it should be applied.

I also added new tests for ilp32 mode, and the problem is that some fail. So I'd
like to ask for help here.
Namely, 3 new tests are failed. tls-relax-large-desc-ie-ilp32 and
tls-relax-large-desc-le-ilp32 fail due to error "cannot represent
BFD_RELOC_AARCH64_TLSDESC_OFF_G1 relocation in this object file format", and
test tls-desc-ie-ilp32 fails due to weird "undefined reference to `v2".

If my understanding is correct, this error is because GAS cannot translate
assembler commands to machine codes properly. If so, it may happen with some real
program one day.

It may also be my misunderstanding on how gas/ld work.
The "undefined reference to v2" is caused by missing of "-shared" in your linker command.
Thank you, it works. My fail.

BFD_RELOC_AARCH64_TLSDESC_OFF_G1 and the several other relocations in the
error message are designed to be used by large code model only, so they are
not expected to be generated under ILP32.
Hmm... Frankly, I don't understand how to proceed with that. The
simplest way is to drop that couple of tests and resend the patch.

But for me there's a problem here. If some asm file will contain the
instruction sequence like in those tests, compilation will fail. It
may come, say, from C inline asm, or whatsoever...

Maybe we should wrap that relocations with "#if ARCH_SIZE == 64 ... #endif"
like it has been done for BFD_RELOC_AARCH64_TLSG_MOW_G1 and
That looks correct to me, simply move those existed relocations into the existed ARCH_SIZE == 64 check block.
Simple move into "ARCH_SIZE == 64" block doesn't work. At least, tests still
produce that errors.

Sure, I mean we still need to move those large model TLS decriptor relocs, for example BFD_RELOC_AARCH64_TLSDESC_OFF_G1, to ARCH_SIZE == 64 block to parallel their TLS trad counterparts, although this is another issue and can be a seperate patch.

Now I'm thinking to drop tls-relax-large-desc-ie-ilp32
and tls-relax-large-desc-le-ilp32, and resend the patch, and then get deeper
into the problem. Is the rest of the patch OK with this change?

I feel there are too many ifdef macro checks and a few duplicated code after this patch.

LP64 and ILP32 generally share the same relaxation strategy, so I would prefer to center the checks into one place for example some thing like:

#if #if ARCH_SIZE == 64
#define movz_r0 0xd2a00000
#define add_r0_r0 0x91000000
#define movz_r0 0x52a00000
#define add_r0_r0 0x11000000

then in the code,

bfd_putl32 (movz_r0/add_r0_r0, ...

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