[PATCH] D34347: [PowerPC] fix potential verification error on __tls_get_addr

[Hiroshi Inoue via Phabricator via llvm-commits]

inouehrs added inline comments.


================
Comment at: lib/Target/PowerPC/PPCTLSDynamicCall.cpp:61
         MachineInstr &MI = *I;
+        if (MI.getOpcode() == PPC::ADJCALLSTACKDOWN) {
+          NeedFence = false;
----------------
nemanjai wrote:
> If the `ADJCALLSTACKDOWN` instruction is guaranteed to occur before any of the "TLS" pseudos in a basic block, that should be mentioned in a comment here. If on the other hand it isn't guaranteed to happen, we can form adjacent `ADJCALLSTACKDOWN/ADJCALLSTACKUP` blocks.
> 
> I don't imagine that's a problem, but it is somewhat inconsistent here...
> What I mean is that if this loop encounters an `ADJCALLSTACKDOWN` which is followed by an `ADJCALLSTACKUP`, we will still not emit the instructions for the subsequent TLS instructions. I am not sure if that is a problem or not.
> 
> So the crux of what I'm saying here is this:
> ```
> ...
> ADJCALLSTACKDOWN
> ...
> ADJCALLSTACKUP
> ...
> ADDItlsgdLADDR
> ```
> Will not add the adjustments. Whereas this will (for all of the TLS instructions):
> ```
> ...
> ADDItlsgdLADDR
> ...
> ADDItlsgdLADDR
> ```
> And this will add the adjustments only for the TLS instructions that appear before the stack adjustments that were already there:
> ```
> ...
> ADDItlsgdLADDR
> ...
> ADJCALLSTACKDOWN
> ...
> ADJCALLSTACKUP
> ...
> ADDItlsgdLADDR
> ```
I modified the logic to be more consistent. Now the following two cases both generate the adjustments.

```
...
ADJCALLSTACKDOWN
...
ADJCALLSTACKUP
...
ADDItlsgdLADDR
```
and
```
...
ADDItlsgdLADDR
...
ADDItlsgdLADDR
```



https://reviews.llvm.org/D34347