on 2022/1/13 上午11:44, David Edelsohn wrote: > On Wed, Jan 12, 2022 at 10:38 PM Kewen.Lin <li...@linux.ibm.com> wrote: >> >> Hi David, >> >> on 2022/1/13 上午11:07, David Edelsohn wrote: >>> On Wed, Jan 12, 2022 at 8:56 PM Kewen.Lin <li...@linux.ibm.com> wrote: >>>> >>>> Hi, >>>> >>>> This patch is to fix register constraint v with >>>> rs6000_constraints[RS6000_CONSTRAINT_v] instead of ALTIVEC_REGS, >>>> just like some other existing register constraints with >>>> RS6000_CONSTRAINT_*. >>>> >>>> I happened to see this and hope it's not intentional and just >>>> got neglected. >>>> >>>> Bootstrapped and regtested on powerpc64le-linux-gnu P9 and >>>> powerpc64-linux-gnu P8. >>>> >>>> Is it ok for trunk? >>> >>> Why do you want to make this change? >>> >>> rs6000_constraints[RS6000_CONSTRAINT_v] = ALTIVEC_REGS; >>> >>> but all of the patterns that use a "v" constraint are (or should be) >>> protected by TARGET_ALTIVEC, or some final condition that only is >>> active for TARGET_ALTIVEC. The other constraints are conditionally >>> set because they can be used in a pattern with multiple alternatives >>> where the pattern itself is active but some of the constraints >>> correspond to NO_REGS when some instruction variants for VSX is not >>> enabled. >>> >> >> Good point! Thanks for the explanation. >> >>> The change isn't wrong, but it doesn't correct a bug and provides no >>> additional benefit nor clarty that I can see. >>> >> >> The original intention is to make it consistent with the other existing >> register constraints with RS6000_CONSTRAINT_*, otherwise it looks a bit >> weird (like was neglected). After you clarified above, RS6000_CONSTRAINT_v >> seems useless at all in the current framework. Do you prefer to remove >> it to avoid any confusions instead? > > It's used in the reg_class, so there may be some heuristic in the GCC > register allocator that cares about the number of registers available > for the target. rs6000_constraints[RS6000_CONSTRAINT_v] is defined > conditionally, so it seems best to leave it as is. >
I may miss something, but I didn't find it's used for the above purposes. If it's best to leave it as is, the proposed patch seems to offer better readability. BR, Kewen > Thanks, David >
