[PATCH] D58400: [AMDGPU] Implement AMDGPUMCInstrAnalysis

[Scott Linder via Phabricator via llvm-commits]

scott.linder marked an inline comment as done.
scott.linder added inline comments.


================
Comment at: test/MC/AMDGPU/branch-comment.s:9
+// BIN-NOT: loop_start:
+// BIN: s_branch 65535 // 000000000004: BF82FFFF <keep_symbol+0x4>
+
----------------
arsenm wrote:
> This should be printed as signed?
I went back to make this change, but I'm not really certain why we choose to represent the branch immediate the way we currently do.

It seems like we reinterpret the bytes of a signed int16 as a signed int64 without a sign-extension when creating the MCOperand immediate. It seems confusing to me, and leads to some awkward casts in places. Would sign-extending and adding checks for `isInt<16>(Imm)` in places be a better approach, or is there something I'm missing?

Alternatively should we just make the asm parser/printer aware of this so we see "-1"? Should we also support the old way? It seems like there must be assembly kernels out there with the old "notation" for negative immediates.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58400