[PATCH] D106445: [amdgpu] Add 64-bit PC support when expanding unconditional branches.

[Michael Liao via Phabricator via llvm-commits]

hliao added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:2307
   RS->setRegUsed(Scav);
 
+  // Now, the distance could be defined.
----------------
arsenm wrote:
> This is supposed to return the number of bytes added, not necessarily the size of the block. At least should assert the block is empty, which I think is always the case 
the assertion on the empty block is already added on L2217. That's how I was motivated to get this change.


================
Comment at: llvm/test/CodeGen/AMDGPU/branch-relaxation-debug-info.mir:12
+# GCN-NEXT: [[POST_GETPC:.Lpost_getpc[0-9]+]]:{{$}}
+# GCN-NEXT:    s_add_u32 s[[PC_LO]], s[[PC_LO]], (BB0_4-[[POST_GETPC]])&4294967295
+# GCN-NEXT:    s_addc_u32 s[[PC_HI]], s[[PC_HI]], (BB0_4-[[POST_GETPC]])>>32
----------------
arsenm wrote:
> It would be better if this mask was printed as hex, but I guess there’s no direct control over this
yes, there is no control on that as the readability depends on the value itself. People may prefer small values in decimal but larger one or special values in hex. But, the definition on small, large, or special values is quite diverse.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106445