Skip to content

[SPARC64] Preserve registers used during optimization transforms#1528

Merged
rui314 merged 2 commits intorui314:mainfrom
koachan:main
Nov 2, 2025
Merged

[SPARC64] Preserve registers used during optimization transforms#1528
rui314 merged 2 commits intorui314:mainfrom
koachan:main

Conversation

@koachan
Copy link
Contributor

@koachan koachan commented Nov 1, 2025

GCC might decide to put the result of the operation in another register, so remove hardcodings of %o0 as the destination register (rd field) during the transform of those relocations.

@koachan
Copy link
Contributor Author

koachan commented Nov 1, 2025

To elaborate more, I'm observing that GCC could emit something like this:

  28:   21 00 00 00     sethi  %hi(0), %l0
                        28: R_SPARC_TLS_LDM_HI22        CurrentThreadTaskGroups
  30:   a0 04 20 00     add  %l0, 0, %l0
                        30: R_SPARC_TLS_LDM_LO10        CurrentThreadTaskGroups
  38:   23 00 00 00     sethi  %hi(0), %l1
                        38: R_SPARC_TLS_LDO_HIX22       CurrentThreadTaskGroups
  48:   a0 05 c0 10     add  %l7, %l0, %l0
                        48: R_SPARC_TLS_LDM_ADD         CurrentThreadTaskGroups
  4c:   a2 1c 60 00     xor  %l1, 0, %l1
                        4c: R_SPARC_TLS_LDO_LOX10       CurrentThreadTaskGroups
 214:   40 00 00 00     call  __tls_get_addr
                        214: R_SPARC_TLS_LDM_CALL       CurrentThreadTaskGroups
 218:   90 10 00 10     mov  %l0, %o0                   ! Set arguments to the call; note how this isn't marked with any relocations.
 21c:   a6 02 00 11     add  %o0, %l1, %l3
                        21c: R_SPARC_TLS_LDO_ADD        CurrentThreadTaskGroups

With the call instruction intact, this is not a problem since __tls_get_addr will take its argument from %o0 and put its return value in %o0 too.
However, with LD->LE optimization pass, the call instruction becomes a nop, which causes the mov to clobber %o0 with an indeterminate value, which causes %l3 to contain a wrong value too - this causes segfaults when the program later tries to access memory pointed to by %l3.

Unfortunately I still don't know how to intentionally trigger this lowering.

@koachan koachan changed the title [SPARC64] Turn the delay slot of R_SPARC_TLS_LDM_CALL to nop if it sets %o0 [WIP][SPARC64] Preserve rd of TLS *_ADD relocations during optimization Nov 1, 2025
@koachan
Copy link
Contributor Author

koachan commented Nov 1, 2025

Just noticed something. GCC might insert uses of %o0 in between R_SPARC_TLS_LDM_ADD and the corresponding R_SPARC_TLS_LDM_CALL.
I think it's better to change approach and let the optimization of R_SPARC_TLS_LDM_ADD (and similar ones for R_SPARC_TLS_GD_ADD) respect the original rd value.

GCC might decide to put the result of the operation in another register,
so remove hardcodings of %o0 as the destination register (`rd` field).
@rui314
Copy link
Owner

rui314 commented Nov 2, 2025

Is this ready to review? I think this change looks at least harmless.

@koachan
Copy link
Contributor Author

koachan commented Nov 2, 2025

Is this ready to review? I think this change looks at least harmless.

Yes, but one question: I've made another commit to remove hardcoded register usage, should I submit it as part of this PR or should I make another PR for it?

The compiler might place the two values on different registers,
so don't modify them during transforms.
@koachan koachan changed the title [WIP][SPARC64] Preserve rd of TLS *_ADD relocations during optimization [SPARC64] Preserve rd of TLS *_ADD relocations during optimization Nov 2, 2025
@koachan koachan changed the title [SPARC64] Preserve rd of TLS *_ADD relocations during optimization [SPARC64] Preserve registers used during optimization transforms Nov 2, 2025
@koachan
Copy link
Contributor Author

koachan commented Nov 2, 2025

Okay I think it should be ready for review now.

@rui314 rui314 merged commit 5cac744 into rui314:main Nov 2, 2025
17 checks passed
@rui314
Copy link
Owner

rui314 commented Nov 2, 2025

Thanks. Regarding the code review process, I’m not too concerned about it because this is after all just a small project. I’ll commit follow-up patches if necessary, so no worries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants