[PATCH] D80713: [AMDGPU] Support disassembly for AMDGPU kernel descriptors
Ronak Chauhan via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Sep 7 04:48:51 PDT 2020
rochauha marked an inline comment as done.
rochauha added inline comments.
================
Comment at: llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp:1542-1548
+ while (C && C.tell() < Bytes.size()) {
+ MCDisassembler::DecodeStatus Status =
+ decodeKernelDescriptorDirective(C, Bytes, KdStream);
+
+ if (Status == MCDisassembler::Fail)
+ return MCDisassembler::Fail;
+ }
----------------
jhenderson wrote:
> If this loop terminates due to `C` being invalid, you don't want to fall out the bottom and return `Success`, I think. You'd want to check `C` after loop termination and return `Fail`. Alternatively, if you return `Fail` from the `decodeKernelDescriptorDirective` you can do `cantFail(C.takeError());` after the loop.
>
> Ideally, we'd actually report the error from `C` in the event of failure, but currently there's no way of communicating that back up to the caller.
Thanks for the pointer! I think `cantFail` seems to be all that is needed, because the cursor was holding Error::success in some cases, which needs to be 'checked' before moving further.
Since the kernel descriptor is well defined, all cases where failure needs to be handled are handled using `MCDisassembler::Fail`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80713/new/
https://reviews.llvm.org/D80713
More information about the llvm-commits
mailing list