[PATCH] D153788: [SystemZ][z/OS] z/OS ADA codegen and emission

Ulrich Weigand via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 30 07:32:08 PDT 2023


uweigand added inline comments.


================
Comment at: llvm/lib/MC/MCObjectFileInfo.cpp:252
       Ctx->getMachOSection("__DWARF", "__debug_frame", MachO::S_ATTR_DEBUG,
                            SectionKind::getMetadata(), "section_frame");
   DwarfPubNamesSection =
----------------
Why are you changing anything about MachO in this patch?


================
Comment at: llvm/lib/Target/SystemZ/SystemZOperands.td:664
+// A symbol in the ADA (z/OS only).
+def adasym : Operand<iPTR>;
----------------
Please move this into SystemZInstrInfo.td next to the place where it is used - just like it is done for `tlssym`.

Also, we're not current using `iPTR` anywhere in the SystemZ back-end and should not start just for this.  Simply use `i64`.


================
Comment at: llvm/lib/Target/SystemZ/SystemZSubtarget.cpp:117
   // FIXME: Explicitly check for functions: the datalayout is currently
   // missing information about function pointers.
+
----------------
This comment should stay with the "if" below that actually refers to the datalayout.


================
Comment at: llvm/lib/Target/SystemZ/SystemZSubtarget.cpp:120
+  if (isTargetzOS())
+    return !isAddressedViaADA(GV);
+
----------------
Factoring this out into a separate routine looks like an unrelated change - I guess I'd be fine with this either way.


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

https://reviews.llvm.org/D153788



More information about the llvm-commits mailing list