[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