On 04.06.19 15:00, Richard Henderson wrote:
> On 6/4/19 4:36 AM, David Hildenbrand wrote:
>> + if (s390_has_feat(S390_FEAT_ESAN3)) {
>> + hwcap |= HWCAP_S390_ESAN3;
>> + }
>> + if (s390_has_feat(S390_FEAT_ZARCH)) {
>> + hwcap |= HWCAP_S390_ZARCH;
>> + }
>
> While it's nice and symetrical testing these two features, I don't think they
> can ever be false.
>
>> + if (s390_has_feat(S390_FEAT_STFLE)) {
>> + hwcap |= HWCAP_S390_STFLE;
>> + }
>> + if (s390_has_feat(S390_FEAT_MSA)) {
>> + hwcap |= HWCAP_S390_MSA;
>> + }
>> + if (s390_has_feat(S390_FEAT_LONG_DISPLACEMENT)) {
>> + hwcap |= HWCAP_S390_LDISP;
>> + }
>> + if (s390_has_feat(S390_FEAT_EXTENDED_IMMEDIATE)) {
>> + hwcap |= HWCAP_S390_EIMM;
>> + }
>> + if (s390_has_feat(S390_FEAT_EXTENDED_TRANSLATION_3) &&
>> + s390_has_feat(S390_FEAT_ETF3_ENH)) {
>> + hwcap |= HWCAP_S390_ETF3EH;
>> + }
>> + /* 31-bit processes can use 64-bit registers */
>> + hwcap |= HWCAP_S390_HIGH_GPRS;
>
> And certainly this could never be set unless ZARCH, otherwise you have no
> 64-bit registers. ;-)
>
> So maybe clearer to just start with
>
> hwcap = HWCAP_S390_ESAN3 | HWCAP_S390_ZARCH | HWCAP_S390_HIGH_GPRS;
Makes sense, and fits into a single line ;)
Thanks!
>
> and continue from there.
>
> Otherwise,
> Reviewed-by: Richard Henderson <richard.hender...@linaro.org>
>
>
> r~
>
--
Thanks,
David / dhildenb
