> -----Original Message----- > From: Segher Boessenkool <seg...@kernel.crashing.org> > Sent: Thursday, October 20, 2022 5:14 AM > To: Andrew Pinski <pins...@gmail.com> > Cc: Jiang, Haochen <haochen.ji...@intel.com>; gcc-patches@gcc.gnu.org; > aol...@gcc.gnu.org; richard.sandif...@arm.com; uweig...@de.ibm.com; > li...@gcc.gnu.org; g...@amylaar.uk; dje....@gmail.com; > olege...@gcc.gnu.org; claz...@synopsys.com; mfort...@gmail.com; > da...@redhat.com; dave.ang...@bell.net; hubi...@ucw.cz; > richard.earns...@arm.com; rguent...@suse.de; > marcus.shawcr...@arm.com; ramana.radhakrish...@arm.com; Liu, Hongtao > <hongtao....@intel.com> > Subject: Re: [PATCH 1/2] Add a parameter for the builtin function of prefetch > to align with LLVM > > On Wed, Oct 19, 2022 at 10:14:28AM -0700, Andrew Pinski wrote: > > Do the testcases really need to be changed rather than adding new > testcases? > > Usually it is better if the testcases not change unless really needed > > to be. That is do these testcases pass without being changed? If not > > this seems not backwards compatible change and is not something which > > we should do. Otherwise you should just add new testcases instead. > > Yes, that is another reason why adding parameters to random builtins is not a > good idea :-) s/random/only vaguely related/, if you want. > > This also makes all existing code using these builtins invalid. If you need > such > testcase changes, that is a red flag. >
Maybe the testcase change cause some misunderstanding and concern. Actually, the patch did not disrupt the previous builtins, as the builtin_prefetch uses vargs. I set the default value of the new parameter as data prefetch, which means that if we are not using the fourth parameter, just like how we use prefetch previously, it is still what it is. The reason why I did the most of the testcase change is to make it looks more completed at the parameter side. I could take back that change on adding parameter in current testcases just add tests related to new parameter, which is a minimal change to current test I suppose. BRs, Haochen > > Segher
