on 2024/6/17 10:31, Peter Bergner wrote: > On 6/16/24 9:10 PM, Kewen.Lin wrote: >> on 2024/6/15 01:05, Peter Bergner wrote: >>> 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! > > I assume this means you're "OK" with the updated patch, correct?
Yes, OK for trunk, thanks! >>> 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? > > I like it, since if Altivec is disabled, having TARGET_ALTIVEC_ABI enabled > makes no > sense to me. That is orthogonal to this bug though, so should be a separate > patch. Yes. > Do you want to take a stab at writing that or do you want me to do that? Either is fine for me, then let me give it a shot. BR, Kewen
