on 2024/6/15 01:05, Peter Bergner wrote: > On 6/13/24 10:26 PM, Peter Bergner wrote: >> On 6/13/24 9:26 PM, Kewen.Lin wrote: >>>>> I understand this is just copied from the if arm, but if I read this >>>>> right, it can be >>>>> simplified as: >>>> >>>> Ok, I'll retest with that simplification. >> >> So I retested a normal powerpc64le-linux build (ie, we default to Power8 >> with Altivec) and it bootstrapped and regtested with no regressions. >> I then attempted a --with-cpu=power5 build to test the non-altivec path, >> but both the unpatched and patched builds died building libgfortran with >> the following error: "error: ‘_Float128’ is not supported on this target". >> I believe that is related to PR113652. I'll kick off the build again, >> this time disabling Fortran and seeing if the build completes. > > My bad for calling the --with-cpu=power5 bootstrap build on ELFv2 a "bug". > It's not, since ELFv2 mandates a cpu with at least ISA 2.07 (eg. Power8) > support and some of the libgfortran code was written assuming that, so what > I was trying to do was really not supported (ie, luser error). > > That said, the --with-cpu=power5 build without fortran did bootstrap and > regtest with no regressions, so the build did test that code path and > exposed no problems.
OK, nice! Thanks! > > > >>>> That's what I expected too! :-) However, I was surprised to learn that >>>> -mno-altivec >>>> does *not* disable TARGET_ALTIVEC_ABI. I had to explicitly use the -mabi= >>>> option to >>>> expose the bug. >>> >>> oh, it's surprising, I learn something today! :) I guess it's not >>> intentional but just no >>> one noticed it, as it seems nonsense to have altivec ABI extension but not >>> using any altivec >>> features. > > Currently, TARGET_ALTIVEC_ABI is defined as: > > #define TARGET_ALTIVEC_ABI rs6000_altivec_abi > > Would it make sense to redine it to: > > #define TARGET_ALTIVEC_ABI (TARGET_ALTIVEC && rs6000_altivec_abi) > > ...or add some code in rs6000 option handling to disable rs6000_altivec_abi > when TARGET_ALTIVEC is false? ....or do we care enough to even change it? :-) Assuming the current code is robust enough (perfectly guarded by some altivec related condition like this altivec register saving slot), there may not any actual errors, but considering not surprising people, I'm inclined to add some option handlings for it, like unsetting rs6000_altivec_abi if !TARGET_ALTIVEC and give some warning if it's explicitly specified, what do you think? BR, Kewen
