This is an archive of the discontinued LLVM Phabricator instance.

[mips] Fix relocation record format and ELF header for N32 ABI
ClosedPublic

Authored by atanasyan on Sep 17 2017, 1:48 AM.

Details

Summary

The N32 ABI uses RELA relocation format, do not use 3-in-1 relocation's encoding, and uses ELFCLASS32. This change passes the IsN32 flag to the MCAsmBackend to distinguish usage of N32 ABI.

We still do not handle some cases like providing the -target-abi=o32 command line option with the mips64 target triple. That's why elf_header.s contains some "FIXME" strings. This case will be fixed in a separate patch.

Diff Detail

Repository
rL LLVM

Event Timeline

atanasyan created this revision.Sep 17 2017, 1:48 AM
sdardis added inline comments.Sep 19 2017, 9:03 AM
test/MC/Mips/cpsetup.s
37

FIXME: this is actually the expansion of .cpsetup for -mno-shared. N32 in -mshared mode generates the same 3-in-1 relocation sequence as N64.

See my comments on https://reviews.llvm.org/D21131

test/MC/Mips/elf_header.s
75–77

The N64 check prefix for EF_MIPS_PC looks to be spurious here, as llvm-mc is generating non-position independent code as it's an empty file.

I think that portion of the test (line 136) should be dropped and other tests should be put to catalogue how N64 deals with PIC.

atanasyan added inline comments.Sep 19 2017, 11:30 AM
test/MC/Mips/cpsetup.s
37
  • Does it really generates 3-in-1 relocation where three relocations are packed into the single relocation record? As far as I understand N32 ABI uses for that a series of relocations targeted the same place in the code. For example this code:
  .globl  foo
  .ent  foo
foo:
  lui   $gp, %hi(%neg(%gp_rel(foo+4)))
  addiu $gp, $gp, %lo(%neg(%gp_rel(foo+4)))
  daddu $gp, $gp, $25
  .end  foo

generates the following relocations:

Relocations [
  Section (3) .rela.text {
    0x0 R_MIPS_GPREL16 foo 0x4
    0x0 R_MIPS_SUB - 0x0
    0x0 R_MIPS_HI16 - 0x0
    0x4 R_MIPS_GPREL16 foo 0x4
    0x4 R_MIPS_SUB - 0x0
    0x4 R_MIPS_LO16 - 0x0
  }
  • Do you suggest to fix assemble code in the test case and/or write a comment or something else? BTW my next patch fixes relocation generation for the code like lui $gp, %hi(%neg(%gp_rel(foo+4))) in case of N32 ABI.
test/MC/Mips/elf_header.s
75–77

OK

sdardis added inline comments.Sep 20 2017, 5:46 AM
test/MC/Mips/cpsetup.s
37
  • Apologies, I managed to confuse myself over the 3-in-1 relocation record format and the differences between -mshared / -mno-shared for n32/n64.

-mno-shared for n32 will generate the sequence for .cpsetup $25, 8, __cerror:

 0: 08 00 bc ff   sd  $gp, 8($sp)
 4: 00 00 1c 3c   lui $gp, 0
00000004:  R_MIPS_HI16  __gnu_local_gp
 8: 00 00 9c 27   addiu $gp, $gp, 0
00000008:  R_MIPS_LO16  __gnu_local_gp

-mshared for n32 will generate for the following for the same pseudo-op:

   0:	ffbc0008 	sd	gp,8(sp)
   4:	3c1c0000 	lui	gp,0x0
			4: R_MIPS_GPREL16	__cerror
			4: R_MIPS_SUB	*ABS*
			4: R_MIPS_HI16	*ABS*
   8:	279c0000 	addiu	gp,gp,0
			8: R_MIPS_GPREL16	__cerror
			8: R_MIPS_SUB	*ABS*
			8: R_MIPS_LO16	*ABS*
   c:	0399e021 	addu	gp,gp,t9

Which is the same series of relocs that is used by n64 in both -mshared/-mno-shared. N32 encodes the relocations as 6 entries and n64 encodes them as 2 3-in-1 relocations.

  • The assembly is fine, I wanted a comment pointing out that the sequence being tested for is the one for the -mno-shared case, and that the -mshared case produces the same series of relocations as the N64 cases.

Addressed review comment:

  • Add FIXME comment to the cpsetup.s that we implicitly check the -mno-shared case only.
  • Enable more checks in the elf_header.s and delete invalid checking for the ELF_MIPS_PIC flag.
sdardis accepted this revision.Sep 21 2017, 2:26 AM

LGTM with inline nit addressed.

test/MC/Mips/elf_header.s
112

You've retained the N64 check prefix in the relevant tests, can you add N64-NOT: for the EF_MIPS_ABI2 / EF_MIPS_ABI_O32 / EF_MIPS32BITMODE flags?

This revision is now accepted and ready to land.Sep 21 2017, 2:26 AM
This revision was automatically updated to reflect the committed changes.

Thanks for review.