[PATCH] D101178: [XCOFF][AIX] Add Global Variables Directly to TOC for 32 bit AIX

[Sean Fertile via Phabricator via llvm-commits]

sfertile added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:484
+
+  assert(!(GV->hasCommonLinkage() || GV->hasLocalLinkage() ||
+           GV->hasPrivateLinkage()) &&
----------------
sfertile wrote:
> I would split out 'hasCommonLinkage()` into its own assert with the string saying something along the lines of `tentative definitions cannot have mapping class XMC_TD`. 
> 
> The local linkage limitation I believe we hit when compiling spec right? In that case maybe change the message to 'local linkage globals cannot have mapping class XMC_TD`. If we didn't, then make sure to test with the system assembler first to make sure it is invalid to define a local linkage global in the .toc.
I did some poking, and the problem is that clang use .lcomm to emit local linkage for internal/private linkage globals. I believe we will be able to support local linkage toc-data by skipping the lcomm directive like


```
     .csect i[TD],2
      .align 2
      .vbyte 4, <val>  
```

So make the local/private linkage a fatal error instead of an assert (since its going to be a temporary limitation)


================
Comment at: llvm/test/CodeGen/PowerPC/toc-data.ll:73
+
+define dso_local i32 @read_i32_linkage() {
+  entry:
----------------
minor nit: name should be `read_i32_local_linkage()`

Also please move this into its own file, and put the toc-data attribute on ilocal and check for an error message.


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

https://reviews.llvm.org/D101178