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. Thanks, David
