Alexandre Oliva <ol...@adacore.com> writes:
> On Feb 26, 2021, 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.
>
> I think the feedback is missing the point of the obvious bug that Eric's
> patch fixes.
>
> We have a dejagnu proc that should return any target-specific options
> necessary for a sqrt insn to be available:
>
> # Return any additional options to enable square root intructions.

Agreed FWIW.  The intention of dg-add-options was to try to increase
test coverage by supporting pairs of procs, one to answer “can I enable
this feature?” and another to say “this is how to enable the feature”.
The question isn't whether sqrt is already enabled, but whether it can be.

Both proposed fixes are correct, in that they make the pairs of procs
consistent.  Changing add_options_for_sqrt_insn is more in the spirit
of how this was supposed to work though.

Thanks,
Richard

Reply via email to