[PATCH] D158197: [PowerPC][lld] Account for additional X-Forms -> D-Form/DS-Forms load/stores when relaxing initial-exec to local-exec

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: lld/ELF/Arch/PPC64.cpp:849
+  case LWAX:
+    return ((LWA << 26) | 0x2);
+  case LDX:
----------------
ditto below


================
Comment at: lld/ELF/Arch/PPC64.cpp:855
+  default:
+    return 1;
+  }
----------------
amyk wrote:
> nemanjai wrote:
> > I forgot to mention it yesterday, but I don't think we should have a different signal for failure here (i.e. returning 1 instead of 0 like in `getPPCDFormOp()`). Let's just use zero for both.
> I will update this.
Mark as unresolved.


================
Comment at: lld/ELF/Arch/PPC64.cpp:862
   case LBZX:
-    return LBZ;
+    return (LBZ << 26);
   case LHZX:
----------------
ditto below


================
Comment at: lld/test/ELF/ppc64-tls-ie.s:157
 
+# LE-LABEL: <test11>:
+# LE-NEXT:  nop
----------------
Preferred style for new tests is that instructions are indented 2 column more than `<test11>`:
```
# LE-LABEL: <test11>:
# LE-NEXT:    nop
# LE-NEXT:    addis 3, 13, 0
# LE-NEXT:    lha 3, -28670(3)
```


================
Comment at: lld/test/ELF/ppc64-tls-ie.s:161
+# LE-NEXT: lha 3, -28670(3)
+test11:
+  addis 3, 2, s at got@tprel at ha
----------------
Consider changing the numbering to something more descriptive. Then, if we want to add a new test in the middle, we won't have to renumber all following `test*`.


================
Comment at: lld/test/ELF/ppc64-tls-ie.s:166
+
+# LE-LABEL: <test12>:
+# LE-NEXT:  nop
----------------
ditto


================
Comment at: lld/test/ELF/ppc64-tls-ie.s:175
+
+# LE-LABEL: <test13>:
+# LE-NEXT:  nop
----------------
ditto


================
Comment at: lld/test/ELF/ppc64-tls-pcrel-ie.s:158
+.section .text_incrval2, "ax", %progbits
+IEIncrementVal2:
+	pld 4, y at got@tprel at pcrel(0), 1
----------------
Consider changing the numbered section name and label name to something more descriptive. Then, if we want to add a new test in the middle, we won't have to renumber all following `.text_incrval*` and `IEIncrementVal*`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158197