[lldb][nfc] Prepare OperatingSystemSwiftTasks for vectorized memory reads part 2 #11676
Conversation
felipepiovezan
force-pushed
the
felipe/multimem_prep_swift_p2
branch
from
5a70664 to
67b4dc4
Compare
|
@swift-ci test |
|
@swift-ci test macos platform |
1 similar comment
|
@swift-ci test macos platform |
| /// Propagate input errors directly to the output, forward proper inputs to | ||
| /// `to_read`. | ||
| for (std::optional<addr_t> maybe_ptr : maybe_task_addrs) { | ||
| if (!maybe_ptr) | ||
| final_results.emplace_back(std::nullopt); | ||
| else { | ||
| final_results.push_back(0); | ||
| to_read.push_back(*maybe_ptr + job_id_offset); | ||
| } | ||
| } | ||
|
|
||
| /// TODO: replace this loop with a vectorized memory read. | ||
| llvm::SmallVector<std::optional<addr_t>> read_results; | ||
| for (addr_t task_id_addr : to_read) { | ||
| Status error; | ||
| // The Task ID is at offset job_id_offset from the Task pointer. | ||
| constexpr uint32_t num_bytes_task_id = 4; | ||
| auto task_id = process.ReadUnsignedIntegerFromMemory( | ||
| task_id_addr, num_bytes_task_id, LLDB_INVALID_ADDRESS, error); | ||
| if (error.Fail()) | ||
| read_results.push_back(std::nullopt); | ||
| else | ||
| read_results.push_back(task_id); | ||
| } | ||
|
|
||
| // Move the results into the slots not filled by errors from the input. | ||
| llvm::ArrayRef<std::optional<addr_t>> results_ref = read_results; | ||
| for (std::optional<addr_t> &maybe_result : final_results) | ||
| if (maybe_result) | ||
| maybe_result = results_ref.consume_front(); | ||
| assert(results_ref.empty()); |
There was a problem hiding this comment.
Is the three for loops required for the future version using the multi memory read? As it is here, I see it as more complex than a single for loop, and don't see why it would be three.
There was a problem hiding this comment.
The middle loop is going to be removed: felipepiovezan@41d4977
There was a problem hiding this comment.
The two other loops are just supporting the fact that the inputs may be optional null, but the vectorized memory read cannot handle that, so we need to drop the nullopts and restore them later
There was a problem hiding this comment.
I should emphasize: most of this PR is so that the diff linked above is minimal
| // If this is not a thread running a Task, add it to the list as is. | ||
| if (!task_id) { | ||
| LLDB_LOG(log, "OperatingSystemSwiftTasks: could not get ID of Task {0:x}", | ||
| *task_addr); |
There was a problem hiding this comment.
is this deref of task_addr safe? it seems it could be nullopt here.
There was a problem hiding this comment.
It is, if we had a task id, then we must have had a task addr (that's where we are reading from)
There was a problem hiding this comment.
Oh sorry, my bad, I thought this was a comment on a different line
…eads part 2 This NFC commit converts the helper function FindTaskId into FindTaskIds, operating on an array of task addresses instead of a single task address. This will make the subsequent diff to use a vectorized memory read much smaller.
felipepiovezan
force-pushed
the
felipe/multimem_prep_swift_p2
branch
from
67b4dc4 to
fcab7f1
Compare
|
@swift-ci test |
| FindTaskIds(llvm::ArrayRef<std::optional<addr_t>> maybe_task_addrs, | ||
| Process &process) { | ||
| llvm::SmallVector<std::optional<uint64_t>> final_results; | ||
| llvm::SmallVector<addr_t> to_read; |
There was a problem hiding this comment.
If it's unlikely we hit the small case I would just use a vector here, but in leaf functions the SmallVector doesn't really hurt, so maybe you can ignore this :-)
|
@swift-ci test windows platform |
3 participants
This NFC commit converts the helper function FindTaskId into FindTaskIds, operating on an array of task addresses instead of a single task address.
This will make the subsequent diff to use a vectorized memory read much smaller.