[PATCH] D92058: [XCOFF][AIX] Alternative path in EHStreamer for platforms do not have uleb128 support

Hubert Tong via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 30 19:39:59 PST 2020


hubert.reinterpretcast added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/EHStreamer.cpp:560
+      const unsigned DisplacementBeforeAlign =
+          2 // LP Encoding and Type Encoding.
+          + ByteSizeOfLSDAWithoutAlign + LSDASizeBeforeAlign;
----------------
Use `LPStartEncoding` and `TypeTableEncoding` to match the comments or "LPStart Encoding" and "TType Encoding" to match the assembly annotation.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/EHStreamer.cpp:570
+      // The LSDASizeWithAlign could use 1 byte less padding for alignment
+      // when the data we use to represent the LSDA Size needs to be 1 byte
+      // larger than we previously calcuated without alignment.
----------------
Minor nit: put "needs" in scare quotes because we are using potentially-overlong ULEB128 encodings.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/EHStreamer.cpp:571
+      // when the data we use to represent the LSDA Size needs to be 1 byte
+      // larger than we previously calcuated without alignment.
+      if (ByteSizeOfLSDAWithAlign > ByteSizeOfLSDAWithoutAlign)
----------------
Typo fix.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/EHStreamer.cpp:573
+      if (ByteSizeOfLSDAWithAlign > ByteSizeOfLSDAWithoutAlign)
+        LSDASizeWithAlign = LSDASizeWithAlign - 1;
+
----------------
Minor nit: Use `--` or `-=`.


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

https://reviews.llvm.org/D92058



More information about the llvm-commits mailing list