[PATCH] D144941: [m68k] Add TLS Support

[Min-Yih Hsu via Phabricator via llvm-commits]

myhsu added inline comments.


================
Comment at: llvm/test/CodeGen/M68k/TLS/tlsie.ll:10
+; CHECK-NEXT:  ; %bb.0: ; %entry
+; CHECK-NEXT:    suba.l #4, %sp
+; CHECK-NEXT:    .cfi_def_cfa_offset -8
----------------
0x59616e wrote:
> jrtc27 wrote:
> > Why do we need the stack adjustment? We're not passing any arguments to __m68k_read_tp. Or is this some stack alignment issue to compensate for the implicit push of the return address?
> This adjustment is the consequence of `M68kSubtarget::stackAlignment` [0]
> 
> The comment says:
> ```
>   /// The minimum alignment known to hold of the stack frame on
>   /// entry to the function and which must be maintained by every function.
>   unsigned stackAlignment = 8;
> ```
> 
> I don't know why it is set to 8 as the ABI only requires 2.
> 
> @myhsu do you have any clue ?
> 
> [0] https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/M68k/M68kSubtarget.h#L73
> This adjustment is the consequence of `M68kSubtarget::stackAlignment` [0]
> 
> The comment says:
> ```
>   /// The minimum alignment known to hold of the stack frame on
>   /// entry to the function and which must be maintained by every function.
>   unsigned stackAlignment = 8;
> ```
> 
> I don't know why it is set to 8 as the ABI only requires 2.
> 
> @myhsu do you have any clue ?
> 
> [0] https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/M68k/M68kSubtarget.h#L73

I'm not sure why it was assigned 8 bytes even after reading the change log. But I'm pretty sure we want to change that (in another patch, of course) to the minimum alignment, which is 2 bytes. 


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

https://reviews.llvm.org/D144941