[PATCH] D104644: [AIX][XCOFF] Support 64-bit relocation writing and related tests

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

In D104644#2836029 <https://reviews.llvm.org/D104644#2836029>, @MaryamBen wrote:

> In D104644#2832538 <https://reviews.llvm.org/D104644#2832538>, @jhenderson wrote:
>
>> This patch from the description is about 64-bit relocation support, so I'm not sure why there's so much changing in the testing. You shouldn't need so many tests for ensuring you have 64-bit relocation support, which implies that the existing tests aren't well designed - they are testing things in a too catch-all manner, and should probably be rewritten to test a small subset of functionality each. Furthermore, why does every test need to cover 64-bit behaviour?
>
> Since I had the fatal error report in 64-bit, I couldn't check the file/section header and symbol table tests. Once I removed this part of the code, I had to implement tests even for the other parts.
>
>   report_fatal_error("64-bit XCOFF object files are not supported yet.");
>
> So maybe I could just change the description or do you have another suggestion ?

Could most of the test be rewritten to not require relocations? It seems like relocations shouldn't be necessary in many situations (assuming they work similarly to how they do in ELF). That would allow those tests to be updated as you make the other patches in the test series, which in turn provides accompanying testing for them.



================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:884
         const size_t CsectRelocCount = Csect.Relocations.size();
-        if (CsectRelocCount >= XCOFF::RelocOverflow ||
-            Section->RelocationCount >= XCOFF::RelocOverflow - CsectRelocCount)
+	if (!TargetObjectWriter->is64Bit() &&
+            (CsectRelocCount >= XCOFF::RelocOverflow ||
----------------
Nit: clang-format.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-xcoff-toc.ll:14
+; RUN:     --xcoff-traceback-table=false -filetype=obj -o %t.o < %s
+; RUN: llvm-readobj --syms %t.o | FileCheck --check-prefixes SYM,SYM64 %s
 
----------------
You may wish to add --match-full-lines to here and the equvialent 32-bit FileCheck command. There's the potential for a false pass in lines like the following:

```
; SYM64:         SymbolAlignmentLog2: 3
```
That line could match any of the following output:
```
SymbolAlignmentLog2: 3
SymbolAlignmentLog2: 32
SymbolAlignmentLog2: 32131241251
```

etc.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-xcoff-toc.ll:97
 ; SYM-NEXT:     StorageMappingClass: XMC_UA (0x4)
-; SYM-NEXT:     StabInfoIndex: 0x0
-; SYM-NEXT:     StabSectNum: 0x0
+; SYM32:        StabInfoIndex: 0x0
+; SYM32-NEXT:   StabSectNum: 0x0
----------------
Here and throughout, this should be `SYM32-NEXT` (or `SYM64-NEXT`), right?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104644/new/

https://reviews.llvm.org/D104644