[PATCH] D85225: [Target][AArch64] Allow for char as int8_t in AArch64AsmParser.cpp
Rainer Orth via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Aug 28 15:24:56 PDT 2020
ro added a comment.
In D85225#2244825 <https://reviews.llvm.org/D85225#2244825>, @paulwalker-arm wrote:
> My vote hasn't changed, I mean I already accepted the tweaked Option1 so you could have been done and dusted by now :) I just don't see what's gained from Option3's refactoring. It does the same as Option1 but in a less type safe/flexible/c++ way. Furthermore, the new interface is needlessly different to the other functions that handle SVE immediate values when the vector element type plays a role.
When differerent reviewers expressed different preferences, I guess I expected they would reach some agreement among themselves. But I'm certainly not keen on dragging this along much longer, let alone try yet another variant ;-) Not being much of a C++ programmer, I can't say much one way or another myself. So I guess I'll give it another day or two for others to chime in and then commit the tweaked Option 1 to finally be done with this.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85225/new/
https://reviews.llvm.org/D85225
More information about the llvm-commits
mailing list