diff mbox

[i386] Introduce support for PKU instructions.

Message ID CAFULd4atCh7U1UcmqK9JUUNnfemp3fcYtGp9bAeLUXqijsuFNg@mail.gmail.com
State New
Headers show

Commit Message

Uros Bizjak Dec. 23, 2015, 11:43 a.m. UTC 
On Tue, Dec 22, 2015 at 4:43 PM, Kirill Yukhin <kirill.yukhin@gmail.com> wrote:
> Hello Uroš,
> I (hopefully fixed all of inputs, thanks!
>
> Updated patch for i386.md in the bottom,
> rest patch is the same.
>
> Bootstrap in progress. New tests pass.
>
> Is it ok for trunk if bootstrap will pass?
>
> On 20 Dec 11:56, Uros Bizjak wrote:
>> > +(define_expand "rdpkru"
>> > +  [(set (match_operand:SI 0 "register_operand")
>> > +       (unspec:SI [(const_int 0)] UNSPEC_PKU))
>> > +   (set (reg:SI CX_REG)
>> > +       (const_int 0))
>> > +   (clobber (reg:SI DX_REG))]
>> > +  "TARGET_PKU"
>> > +{
>> > +  emit_move_insn (gen_rtx_REG (SImode, CX_REG), CONST0_RTX (SImode));
>> > +  emit_insn (gen_rdpkru_2 (operands[0]));
>> > +  DONE;
>> > +})
>>
>> You should use "parallel" to emit insn with several parallel
>> expressions. So, in the preparation statements, you move const0 to a
>> pseudo, so the RA will later use correct register. And please leave to
>> the expander to emit the pattern.
>>
>> > +(define_insn "rdpkru_2"
>> > +  [(set (match_operand:SI 0 "register_operand" "=a")
>> > +       (unspec:SI [(const_int 0)] UNSPEC_PKU))
>> > +   (clobber (reg:SI DX_REG))
>> > +   (use (reg:SI CX_REG))]
>> > +  "TARGET_PKU"
>> > +  "rdpkru"
>> > +  [(set_attr "type" "other")])
>>
>> Please do not use explicit hard registers. There are appropriate
>> single-reg constraints available for use. Without seeing the
>> documentation, I think the above should look like:
>>
>> (define_insn "*rdpkru"
>>   [(set (match_operand:SI 0 "register_operand" "=a")
>>        (unspec:SI [(match_operand:SI 1 "register_operand" "c")] UNSPEC_PKU))
>>    (clobber (rmatch_operand "register_operand "=d"))
>>   "TARGET_PKU"
>>   "rdpkru"
>>   [(set_attr "type" "other")])

According to the SDM, rdpkru moves zero to a %edx register. Let's be
precise and model this. Also, since rdpkru insn accesses hidden state
(PKRU that is not modelled properly in the pattern), it should also be
marked as unspec_volatile. I took the liberty and rewrite the i386.md
changes in the attached patch.

Your patch with the attached i386.md changes is OK for mainline, after
additional bootstrap and regression test.

Thanks,
Uros.
diff mbox

Patch

Index: i386.md
===================================================================
--- i386.md	(revision 231927)
+++ i386.md	(working copy)
@@ -268,6 +268,8 @@ 
   ;; For CLZERO support
   UNSPECV_CLZERO
 
+  ;; For RDPKRU and WRPKRU support
+  UNSPECV_PKU
 ])
 
 ;; Constants to represent rounding modes in the ROUND instruction
@@ -19320,6 +19322,48 @@ 
   [(set_attr "type" "imov")
    (set_attr "mode" "<MODE>")])
 
+;; RDPKRU and WRPKRU
+
+(define_expand "rdpkru"
+  [(parallel
+     [(set (match_operand:SI 0 "register_operand")
+	   (unspec_volatile:SI [(match_dup 1)] UNSPECV_PKU))
+      (set (match_dup 2) (const_int 0))])]
+  "TARGET_PKU"
+{
+  operands[1] = force_reg (SImode, const0_rtx);
+  operands[2] = gen_reg_rtx (SImode);
+})
+
+(define_insn "*rdpkru"
+  [(set (match_operand:SI 0 "register_operand" "=a")
+	(unspec_volatile:SI [(match_operand:SI 2 "register_operand" "c")]
+			    UNSPECV_PKU))
+   (set (match_operand:SI 1 "register_operand" "=d")
+	(const_int 0))]
+  "TARGET_PKU"
+  "rdpkru"
+  [(set_attr "type" "other")])
+
+(define_expand "wrpkru"
+  [(unspec_volatile:SI
+     [(match_operand:SI 0 "register_operand")
+      (match_dup 1) (match_dup 2)] UNSPECV_PKU)]
+  "TARGET_PKU"
+{
+  operands[1] = force_reg (SImode, const0_rtx);
+  operands[2] = force_reg (SImode, const0_rtx);
+})
+
+(define_insn "*wrpkru"
+  [(unspec_volatile:SI
+     [(match_operand:SI 0 "register_operand" "a")
+      (match_operand:SI 1 "register_operand" "d")
+      (match_operand:SI 2 "register_operand" "c")] UNSPECV_PKU)]
+  "TARGET_PKU"
+  "wrpkru"
+  [(set_attr "type" "other")])
+
 (include "mmx.md")
 (include "sse.md")
 (include "sync.md")