On Thu, 21 Nov 2013, Cong Hou wrote:
On Thu, Nov 21, 2013 at 4:39 PM, Marc Glisse <marc.gli...@inria.fr> wrote:
On Thu, 21 Nov 2013, Cong Hou wrote:
While I added the new define_insn_and_split for vec_merge, a bug is
exposed: in config/i386/sse.md, [ define_expand "xop_vmfrcz<mode>2" ]
only takes one input, but the corresponding builtin functions have two
inputs, which are shown in i386.c:
{ OPTION_MASK_ISA_XOP, CODE_FOR_xop_vmfrczv4sf2,
"__builtin_ia32_vfrczss", IX86_BUILTIN_VFRCZSS, UNKNOWN,
(int)MULTI_ARG_2_SF },
{ OPTION_MASK_ISA_XOP, CODE_FOR_xop_vmfrczv2df2,
"__builtin_ia32_vfrczsd", IX86_BUILTIN_VFRCZSD, UNKNOWN,
(int)MULTI_ARG_2_DF },
In consequence, the ix86_expand_multi_arg_builtin() function tries to
check two args but based on the define_expand of xop_vmfrcz<mode>2,
the content of insn_data[CODE_FOR_xop_vmfrczv4sf2].operand[2] may be
incorrect (because it only needs one input).
The patch below fixed this issue.
Bootstrapped and tested on ax x86-64 machine. Note that this patch
should be applied before the one I sent earlier (sorry for sending
them in wrong order).
This is PR 56788. Your patch seems strange to me and I don't think it
fixes the real issue, but I'll let more knowledgeable people answer.
Thank you for pointing out the bug report. This patch is not intended
to fix PR56788.
IMHO, if PR56788 was fixed, you wouldn't have this issue, and if PR56788
doesn't get fixed, I'll post a patch to remove _mm_frcz_sd and the
associated builtin, which would solve your issue as well.
For your function:
#include <x86intrin.h>
__m128d f(__m128d x, __m128d y){
return _mm_frcz_sd(x,y);
}
Note that the second parameter is ignored intentionally, but the
prototype of this function contains two parameters. My fix is
explicitly telling GCC that the optab xop_vmfrczv4sf3 should have
three operands instead of two, to let it have the correct information
in insn_data[CODE_FOR_xop_vmfrczv4sf3].operand[2] which is used to
match the type of the second parameter in the builtin function in
ix86_expand_multi_arg_builtin().
I disagree that this is intentional, it is a bug. AFAIK there is no AMD
documentation that could be used as a reference for what _mm_frcz_sd is
supposed to do. The only existing documentations are by Microsoft (which
does *not* ignore the second argument) and by LLVM (which has a single
argument). Whatever we chose for _mm_frcz_sd, the builtin should take a
single argument, and if necessary we'll use 2 builtins to implement
_mm_frcz_sd.
--
Marc Glisse
