[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
Tue Dec 1 13:40:21 PST 2020


hubert.reinterpretcast added a comment.

LGTM with comments.



================
Comment at: llvm/include/llvm/MC/MCAsmInfo.h:189
 
+  /// True if target supports LEB128 directives.
+  bool HasLEB128Directives = true;
----------------
Minor nit: missing "the".


================
Comment at: llvm/lib/MC/MCAsmInfo.cpp:32
+cl::opt<cl::boolOrDefault> UseLEB128Directives(
+    "use-leb128-directive", cl::Hidden,
+    cl::desc(
----------------
Fix typo. Test uses `use-leb128-directives` as intended.


================
Comment at: llvm/lib/MC/MCAsmInfoXCOFF.cpp:26-28
+  HasLEB128Directives = UseLEB128Directives == cl::BOU_UNSET
+                            ? false
+                            : UseLEB128Directives == cl::BOU_TRUE;
----------------
The value to use if not unset is already handled.


================
Comment at: llvm/lib/Target/TargetLoweringObjectFile.cpp:60
+unsigned TargetLoweringObjectFile::getCallSiteEncoding() const {
+  // If target does not have leb128 directives, we would need the
+  // call site encoding to be udata4 so that the alternative path
----------------
Minor nit: style.


================
Comment at: llvm/lib/Target/TargetLoweringObjectFile.cpp:62
+  // call site encoding to be udata4 so that the alternative path
+  // for not having leb128 directives could work.
+  if (!getContext().getAsmInfo()->hasLEB128Directives())
----------------
Same comment.


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

https://reviews.llvm.org/D92058



More information about the llvm-commits mailing list