[PATCH] D76591: [PPCInstPrinter] Change printBranchOperand(calltarget) to print the target address in hexadecimal form

[Sean Fertile via Phabricator via llvm-commits]

sfertile added a comment.

In D76591#1939442 <https://reviews.llvm.org/D76591#1939442>, @MaskRay wrote:

> In D76591#1939318 <https://reviews.llvm.org/D76591#1939318>, @jasonliu wrote:
>
> > @nemanjai 
> >  AIX system assembler does not seem to like this new form.
> >
> >   Assembler:
> >   test.s: line 46: 1252-086 The target of the branch instruction
> >           must be a relocatable or external expression.
> >
>
>
> llvm-objdump -d output may not be assembled back. That said, I think most users like the PC relative immediate to be displayed as the target address. If AIX users don't agree, I can special case it.


Sorry for the churn but I think your initial approach is correct. objdump on AIX also prints the target address as hex and like you say, the output isn't meant to be assembled back. Any cases where we the output is asm are unaltered right?



================
Comment at: llvm/lib/Target/PowerPC/MCTargetDesc/PPCInstPrinter.cpp:438
+      Target &= 0xffffffff;
+    O << formatHex(Target);
+  }
----------------
MaskRay wrote:
> sfertile wrote:
> > Do we have a way to print the hex value without a leading '0x'? If we are making the effort to match binutils objump,  why wouldn't we match exactly for this case?
> Don't use formatHex then we are done. However, I actually think an explicit 0x is better.
> 
> A GNU objdump -d result (x86_64) cannot be re-assembled back. The omission of 0x is one of the reasons.
I don't feel too strongly either way but I though the tools that are replicating binutils tools are meant to be exact drop-in replacements outside of replicating bugs. If this is one of many things that precludes objdumps output  from being disassembled then we should  prioritize compatibility, otherwise I have no reservations about keeping the leading `0x'.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76591