MaskRay wrote: > Not that the patch is especially long/complicated, but could be split into > the refactor/move of the MC function, then the new usage, if you like (usual > reasons - smaller patches are easier to root cause, functionality can be > reverted without thrashing the refactored code (or refactored code can be > reverted if issues are found in that before the usage goes in), etc)
The body of `encodeCrel` is a simple move. Even with a signature change, the two parts (extract and adapt assembler + support llvm-objcopy) could still be considered separate. However, some reviewers might prefer seeing both parts together for a better understanding of the extracted API. Based on the comments from jh7370 and smithp35, the extraction seems reasonable. **How about I landing the extraction part separately after receiving official feedback? I will then rebase this llvm-objcopy patch.** (I maintain patches in a stack and ensure the final one https://github.com/MaskRay/llvm-project/commits/demo-crel/ passes a local integration test. There is some process inconvenience given that the llvm-objdump PR also modifies Object and has been approved yet...) https://github.com/llvm/llvm-project/pull/97521 _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
