[PATCH] D92398: [AIX][XCOFF] emit traceback table for function in aix

[Digger via Phabricator via llvm-commits]

DiggerLin marked 13 inline comments as done.
DiggerLin added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1741
+
+  if (!TM.getXCOFFTracebackTable())
+    return;
----------------
jasonliu wrote:
> nit: a slight preference to have this early return inside of `emitTracebackTable()` function. 
putting if (!TM.getXCOFFTracebackTable()) here, it do not need extra function call emitTracebackTable  when  getXCOFFTracebackTable is false.


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1769
+  CommentString.clear();                                                       \
+  OutStreamer->emitIntValueInHexWithPadding(X, S)
+
----------------
jasonliu wrote:
> DiggerLin wrote:
> > DiggerLin wrote:
> > > jasonliu wrote:
> > > > jasonliu wrote:
> > > > > I don't think it's worth to add this function/macro if it just saves one extra line `OutStreamer->emitIntValueInHexWithPadding(X, S)`. And it's actually more helpful for people to see that extra line and know a byte have been printed. 
> > > > Do you really need to emit these values with Paddings?
> > > I am prefer with paddings.
> > it save two lines. and there are several places invoke OutStreamer->emitIntValueInHexWithPadding(X, S), I think it worth it. if I want to change the format of the output, I only need change the macro , I do not several place, for example, if you do not agree with pad, I only need to change the macro.
> Any reason you need to emit with Paddings though?
> If not, why not just call `emitIntValue` instead.
for example , if the value is 3, if use the emitIntValue. it will be display 
.byte 0x3,
with 
emitIntValueInHexWithPadding will show as  0x03.  I can know , it is only byte value.

if it is two bytes value , it will show as  0x0003.  even there is (.vbytes 2, 0x3), but I prefer show as (.vbytes 2, 0x0003) .



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92398