[PATCH] D60474: [llvm] [lit] Add target-x86 feature

[Petr Hosek via Phabricator via llvm-commits]

phosek added a comment.

In D60474#1460785 <https://reviews.llvm.org/D60474#1460785>, @labath wrote:

> When I requested this, I expected you would go and add something to the lldb lit config files. However, the way you've chosen to implement that seems to complement nicely the existing features in the llvm files, so I think it may go in here as well. But, I'd like for someone from the llvm side to sign off on this as well.
>
> The one question I have is whether we really want to bundle x86 and x86_64 into one feature. It seems to me that if you were to extend the motivating test only slightly (e.g. to include %xmm8-15), then you'd need to differentiate between the 32 and 64 bit variants. OTOH, if these are separate features, then you can always say `REQUIRES: x86 || x86_64` for tests that work on both.


Looks like we raced with @labath, I agree with his response that you can also use `REQUIRES: x86 || x86_64` to achieve the same, so the question is how many of these cases you're going to have in lldb, if it's just handful then it's probably not worth introducing a new feature, but if it's going to be a significant number then a new feature is reasonable.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60474