[PATCH] D100214: [AIX][TLS] Add support for TLSGD relocations to XCOFF objects

Sean Fertile via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon May 3 10:28:04 PDT 2021


sfertile added inline comments.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-tls-xcoff-reloc-large.ll:526
+; DIS:      00000000 (idx: 5) .storesTIInit:
+; DIS-NEXT:        0: 7c 08 02 a6  	mflr 0
+; DIS-NEXT:        4: 90 01 00 08  	stw 0, 8(1)
----------------
I don't know if we have decided on any formatting guidelines for these types of tests so that we have a consistent feel across all the tests in the back end, but if we have let me know and we can ignore this comment.

I think for this output here, the only important part is the instruction, while the instructions that have relocations the important part also includes the instructions address. From that I think should have lit checks along the lines of the following:

```
DIS:              virt-addr (idx: N)    Label:
DIS-NEXT:                                  Instr operands
...
DIS-NEXT:    virt-addr:  {{.*}}     Instr operands
DIS-NEXT:    virt-addr: Reloc (idx: M) Sym_Name
...
```

where we include the virtual address on labels, instructions that are referenced by relocations, and the relocations themselves. In the instructions that are referenced by relocations we need to add a regex to consume the raw data, but in the other instructions we just ignore everything preceding the instruction mnemonic.  


================
Comment at: llvm/test/CodeGen/PowerPC/aix-tls-xcoff-reloc-large.ll:575
+; DIS:      00000088 (idx: 9) GInit[RW]:
+; DIS-NEXT:       88: 3f f0 00 00  	addis 31, 16, 0
+; DIS-NEXT:       8c: 00 00 00 00  	<unknown>
----------------
When we get to the data section, whats important has flipped and the virtual address and the contents of memory is important, while the text that comes after the memory contents typically isn't important (in fact its typically misleading 😆) and so I think we should drop the text following the raw data. 

A minor nit: FWIW I think we should try to align the contents of the checks also. eg:

```
DIS:            00000090 (idx: 11) storesTIInit[DS]:
DIS-NEXT:               90: 00 00 00 00
DIS-NEXT: 00000090: R_POS  (idx: 5) .storesTIInit
...
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100214



More information about the llvm-commits mailing list