Hi Uros, Updated patch attached, please find comments below.
> -----Original Message----- > From: Uros Bizjak [mailto:ubiz...@gmail.com] > Sent: Wednesday, May 9, 2018 1:47 PM > To: Peryt, Sebastian <sebastian.pe...@intel.com> > Cc: gcc-patches@gcc.gnu.org; Kirill Yukhin <kirill.yuk...@gmail.com> > Subject: Re: [PATCH][i386] Adding WAITPKG instructions > > On Tue, May 8, 2018 at 1:34 PM, Peryt, Sebastian <sebastian.pe...@intel.com> > wrote: > > Hi, > > > > This patch adds support for WAITPKG instructions. > > > > Is it ok for trunk and after few day for backport to GCC-8? > > (Removed) > > > > > > + case IX86_BUILTIN_UMONITOR: > + arg0 = CALL_EXPR_ARG (exp, 0); > + op0 = expand_normal (arg0); > + if (!REG_P (op0)) > + op0 = ix86_zero_extend_to_Pmode (op0); > + > + emit_insn (ix86_gen_umonitor (op0)); > + return 0; > > Please see how movdir64b handles its address operand. Also, do not use global > ix86_gen_monitor, just expand directly in the same way as movdir64b. > Fixed. > + case IX86_BUILTIN_UMWAIT: > + case IX86_BUILTIN_TPAUSE: > + rtx eax, edx, op1_lo, op1_hi; > + arg0 = CALL_EXPR_ARG (exp, 0); > + arg1 = CALL_EXPR_ARG (exp, 1); > + op0 = expand_normal (arg0); > + op1 = expand_normal (arg1); > + eax = gen_rtx_REG (SImode, AX_REG); > + edx = gen_rtx_REG (SImode, DX_REG); > + if (!REG_P (op0)) > + op0 = copy_to_mode_reg (SImode, op0); > + if (!REG_P (op1)) > + op1 = copy_to_mode_reg (DImode, op1); > + op1_lo = gen_lowpart (SImode, op1); > + op1_hi = expand_shift (RSHIFT_EXPR, DImode, op1, > + GET_MODE_BITSIZE (SImode), 0, 1); > + op1_hi = convert_modes (SImode, DImode, op1_hi, 1); > + emit_move_insn (eax, op1_lo); > + emit_move_insn (edx, op1_hi); > + emit_insn (fcode == IX86_BUILTIN_UMWAIT > + ? gen_umwait (op0, eax, edx) > + : gen_tpause (op0, eax, edx)); > + > + /* Return current CF value. */ > + op3 = gen_rtx_REG (CCCmode, FLAGS_REG); > + target = gen_rtx_LTU (QImode, op3, const0_rtx); > + > + return target; > > For the above code, please see how xsetbv expansion and patterns are handling > their input operands. There should be two patterns, one for 32bit and the > other > for 64bit targets. The patterns will need to set FLAGS_REG, otherwise the test > will be removed. > I copied what is done for xsetbv expansion and most likely I found some bug in GCC. The problem is that when I use 3 arguments and compile as 64bit version upper part of rax is not cleared. It doesn't appear when I'm using 2 or 4 function arguments. Most likely error is caused by the fact that rdx is used both as an input for function and argument in instruction. When using 3 operands: bar: .LFB5450: .cfi_startproc movq %rdx, %rax umonitor %rdi movq %rdx, %rcx shrq $32, %rcx movq %rcx, %rdx umwait %esi setc %al ret .cfi_endproc When using 4 operands: bar: .LFB5450: .cfi_startproc movl %edx, %esi umonitor %rdi movq %rcx, %rax shrq $32, %rax movq %rax, %rdx movl %ecx, %eax umwait %esi setc %al ret .cfi_endproc Can you please suggest how to proceed here? I cannot open new PR without adding this instruction first. Or maybe you know how to resolve it? > +(define_insn "umwait" > + [(unspec_volatile [(match_operand:SI 0 "register_operand" "r") > + (use (match_operand:SI 1 "register_operand" "a")) > + (use (match_operand:SI 2 "register_operand" "d"))] > + UNSPECV_UMWAIT)] > + "TARGET_WAITPKG" > + "umwait\t{%0}" > + [(set_attr "length" "3")]) > > No need for "use" RTX here and in other patterns. You should also remove {} > from insn template, otherwise there will be no operand printed in some asm > dialect. > Fixed. > Uros. Sebastian
0001-WAITPKG-v2.patch
Description: 0001-WAITPKG-v2.patch
