[PATCH] D95262: [LLD][PowerPC] Fix bug in PC-Relative initial exec

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: lld/test/ELF/ppc64-tls-pcrel-ie.s:82
+# LE-LABEL: <IEAddrCopy>:
+# LE-NEXT:    paddi 3, 13, -28672, 0
+# LE-NEXT:    mr 4, 3
----------------
stefanp wrote:
> nemanjai wrote:
> > MaskRay wrote:
> > > stefanp wrote:
> > > > MaskRay wrote:
> > > > > In this case, does the codegen assume that r3 is killed and cannot be reused?
> > > > This is an artificial test case. There is no real reason why `r4` is used instead of `r3` in the test above. Generating a real test would create more code and it would just clutter the test file. This is a snippet from a real test:
> > > > ```
> > > > 10: 00 00 10 04 00 00 60 e4      	pld 3, 0(0), 1
> > > > 		0000000000000010:  R_PPC64_GOT_TPREL_PCREL34	TGlobal
> > > > 18: 14 6a c3 7f  	add 30, 3, 13
> > > > 		0000000000000019:  R_PPC64_TLS	TGlobal
> > > > 1c: 78 f3 c3 7f  	mr	3, 30
> > > > 20: 01 00 00 48  	bl 0x20
> > > > 		0000000000000020:  R_PPC64_REL24_NOTOC	getConst
> > > > 24: 78 f3 c4 7f  	mr	4, 30
> > > > ```
> > > > the idea is that the `bl` clobbers `r3` and so we save to `r30` first so that we can use the result of the add both before and after the call.
> > > > The only thing I want to test is what happens if the result register of the `add` is different from the result register of the `pld`.
> > > ```
> > > pld 3, 12488(0), 1
> > > add 4, 3, 13
> > > ```
> > > 
> > > r3 != r4. If the compiler generated code makes use of r3, no matter how the linker relaxes the sequence, it will be broken, right?
> > Is it possible for `r3` to be used for anything other than materializing the address of `x`? Basically, the result in `r3` before the relaxation is "The offset of `x` from the thread pointer" and the result in `r4` is "The address of `x`". After the relaxation, both `r3` and `r4` contain "the address of `x`".
> Technically one could create assembly by hand that would break this relaxation.  As Nemanja mentioned we do change what `r3` stores when we relax it.
> 
> However, I do not believe that our compiler can generate code like that. My impression based on the ELFv2 ABI is that `R_PPC64_GOT_TPREL_PCREL34` is only used in code sequences for Initial Exec. If the linker receives this relocation in other situations and `r3` is used in other ways I would consider that bad code gen from the compiler.
> 
> I am basically assuming that all uses of `r3` will be marked with `R_PPC64_TLS` and can be relaxed safely. If that assumption cannot be made we either have to cancel this relaxation completely or we have to add more look-ahead code to check that the add in the TLS pattern always uses the same register as input and output.
I thought there was unactioned items in this comment thread. Can you briefly summarize the compiler behavior and roll that into the description?

I'd like a proof that 

	pld 3, x at got@tprel at pcrel(0), 1
	add 4, 3, x at tls@pcrel

is legitimate compiler emitted code sequence, so relaxing it is a bug. There are other compiler emitted code sequences which are technically really difficult to detect, but compilers do not emit them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95262