[PATCH] D128029: [AArch64] Add target feature "all"

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 24 01:53:27 PDT 2022


MaskRay added a comment.

In D128029#3604070 <https://reviews.llvm.org/D128029#3604070>, @lenary wrote:

> I didn't realise you couldn't nest `any_of` and `all_of` before this patch. Thanks for catching that.
>
> In D128029#3601135 <https://reviews.llvm.org/D128029#3601135>, @peter.smith wrote:
>
>> Main reason for mentioning that is to ask the question; is -mattr=all too low level a concept to model universal disassembly on the command line? I'm thinking it is fine for llvm-mc but perhaps llvm-objdump could do with something a bit more high-level.
>
> I'm not sure how much `-mattr` is a part of the public/backwards-compatible command-line interface for llvm-objdump. I think we could absolutely include an option for this in a command-line interface we commit to not changing, usually the target-specific options are behind `llvm-objdump -M`, which I believe should have enough access to the subtarget to be able to enable features.
>
> The only other suggestion I have apart from the one below is whether we should change the definition of `SysAlias::haveFeatures` in `llvm/lib/Target/AArch64/Utils/AArch64BaseInfo.h` to also check for FeatureAll - I think how those tables are currently structured, they don't use AssemblyPredicates, they use feature bitsets directly, which is maybe not optimal. If you want to leave this refactoring for later, that would make sense to me too.

Thanks. Changing `SysAlias::haveFeatures` is better. Updated.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128029/new/

https://reviews.llvm.org/D128029



More information about the llvm-commits mailing list