On Mon, Mar 01, 2021 at 09:46:29AM +0100, Richard Biener wrote:
> On Sat, Feb 27, 2021 at 2:43 AM Segher Boessenkool
> <seg...@kernel.crashing.org> wrote:
> >
> > On Fri, Feb 26, 2021 at 12:31:16PM -0500, David Edelsohn wrote:
> > > On Fri, Feb 26, 2021 at 11:09 AM Alexandre Oliva <ol...@adacore.com>
> > > wrote:
> > > >
> > > > This patch avoids an ICE in gimplefe-28.c, in our ppc64-vxworks7r2
> > > > tests. Tested on x86_64-linux-gnu, and on the affected platform. Ok to
> > > > install?
> > >
> > > I'm sort of surprised that sqrt instruction would be available for the
> > > target but not enabled by default. Is this really the method that
> > > customers would use? It seems that it might be more appropriate to
> > > xfail or skip the test for PPC64 VxWorks.
> >
> > You should essentially never use -mpowerpc-gpopt, but instead use a
> > -mcpu= that has it. You also of course whould not do that in run tests
> > if the cpu does not support those insns.
> >
> > But, Alex, you say this avoids an ICE? You shouldn't avoid the ICE in
> > that testcase then -- instead, fix it! (Or report it).
>
> The testcase uses a .SQRT direct-internal function and guards itself with
>
> /* { dg-do compile { target sqrt_insn } } */
> /* { dg-options "-fgimple -O2" } */
> /* { dg-add-options sqrt_insn } */
>
> if the power dg setup of sqrt_insn doesn't support this it should be adjusted.
Yeah. It should test _ARCH_PPCSQ. I'll make a patch.
> Using -mcpu=X for sqrt_insn is likely undesirable but for this testcase would
> work (it would make testing with, say, -mcpu=power4 not work).
Works differently on different dejagnu versions, which is why we have
the -mdejagnu=cpu= kludge.
> So I'd
> say as alternative to Alex patch you could adjust
>
> # Return 1 if the target supports hardware square root instructions.
>
> proc check_effective_target_sqrt_insn { } {
> return [check_cached_effective_target sqrt_insn {
> expr { [istarget i?86-*-*] || [istarget x86_64-*-*]
> || [istarget powerpc*-*-*]
> || [istarget aarch64*-*-*]
> || ([istarget arm*-*-*] && [check_effective_target_arm_vfp_ok])
> || ([istarget s390*-*-*]
> && [check_effective_target_s390_vx])
> || [istarget amdgcn-*-*] }}]
> }
>
> to only say yes in case the selected CPU supports sqrt.
Right, replace
|| [istarget powerpc*-*-*]
with
|| [check_effective_target_powerpc_sqrt]
(and define that :-) )
Segher
