[PATCH, i386, Pointer Bounds Checker 31/x] Pointer Bounds Checker builtins for i386 target

Uros Bizjak ubizjak@gmail.com
Tue Sep 16 10:02:00 GMT 2014


> 2014-06-11  Ilya Enkovich  <ilya.enkovich@intel.com>
>
>         * config/i386/i386-builtin-types.def (BND): New.
>         (ULONG): New.
>         (BND_FTYPE_PCVOID_ULONG): New.
>         (VOID_FTYPE_BND_PCVOID): New.
>         (VOID_FTYPE_PCVOID_PCVOID_BND): New.
>         (BND_FTYPE_PCVOID_PCVOID): New.
>         (BND_FTYPE_PCVOID): New.
>         (BND_FTYPE_BND_BND): New.
>         (PVOID_FTYPE_PVOID_PVOID_ULONG): New.
>         (PVOID_FTYPE_PCVOID_BND_ULONG): New.
>         (ULONG_FTYPE_VOID): New.
>         (PVOID_FTYPE_BND): New.
>         * config/i386/i386.c: Include tree-chkp.h, rtl-chkp.h.
>         (ix86_builtins): Add
>         IX86_BUILTIN_BNDMK, IX86_BUILTIN_BNDSTX,
>         IX86_BUILTIN_BNDLDX, IX86_BUILTIN_BNDCL,
>         IX86_BUILTIN_BNDCU, IX86_BUILTIN_BNDRET,
>         IX86_BUILTIN_BNDNARROW, IX86_BUILTIN_BNDINT,
>         IX86_BUILTIN_SIZEOF, IX86_BUILTIN_BNDLOWER,
>         IX86_BUILTIN_BNDUPPER.
>         (builtin_isa): Add leaf_p and nothrow_p fields.
>         (def_builtin): Initialize leaf_p and nothrow_p.
>         (ix86_add_new_builtins): Handle leaf_p and nothrow_p
>         flags.
>         (bdesc_mpx): New.
>         (bdesc_mpx_const): New.
>         (ix86_init_mpx_builtins): New.
>         (ix86_init_builtins): Call ix86_init_mpx_builtins.
>         (ix86_emit_move_max): New.
>         (ix86_expand_builtin): Expand IX86_BUILTIN_BNDMK,
>         IX86_BUILTIN_BNDSTX, IX86_BUILTIN_BNDLDX,
>         IX86_BUILTIN_BNDCL, IX86_BUILTIN_BNDCU,
>         IX86_BUILTIN_BNDRET, IX86_BUILTIN_BNDNARROW,
>         IX86_BUILTIN_BNDINT, IX86_BUILTIN_SIZEOF,
>         IX86_BUILTIN_BNDLOWER, IX86_BUILTIN_BNDUPPER.

Hm, can this patch be compiled as part of the series? The expanders
refer to various gen_bnd patterns that I don't see. Also, I don't see
BND mode introduced.

Anyway, some general observations:

> +    case IX86_BUILTIN_BNDLDX:
> +      arg0 = CALL_EXPR_ARG (exp, 0);
> +      arg1 = CALL_EXPR_ARG (exp, 1);
> +
> +      op0 = expand_normal (arg0);
> +      op1 = expand_normal (arg1);
> +
> +      op0 = force_reg (Pmode, op0);
> +      op1 = force_reg (Pmode, op1);
> +
> +      /* Avoid registers which connot be used as index.  */
> +      if (!index_register_operand (op1, Pmode))
> +       {
> +         rtx temp = gen_reg_rtx (Pmode);
> +         emit_move_insn (temp, op1);
> +         op1 = temp;
> +       }
> +
> +      /* If op1 was a register originally then it may have
> +        mode other than Pmode.  We need to extend in such
> +        case because bndldx may work only with Pmode regs.  */
> +      if (GET_MODE (op1) != Pmode)
> +       op1 = ix86_zero_extend_to_Pmode (op1);
> +
> +      if (REG_P (target))
> +       emit_insn (TARGET_64BIT
> +                  ? gen_bnd64_ldx (target, op0, op1)
> +                  : gen_bnd32_ldx (target, op0, op1));
> +      else
> +       {
> +         rtx temp = gen_reg_rtx (BNDmode);
> +         emit_insn (TARGET_64BIT
> +                    ? gen_bnd64_ldx (temp, op0, op1)
> +                    : gen_bnd32_ldx (temp, op0, op1));
> +         emit_move_insn (target, temp);
> +       }
> +      return target;

I don't like the way arguments are prepared. For the case above,
bnd_ldx should have index_register_operand predicate in its pattern,
and this predicate (and its mode) should be checked in the expander
code. There are many examples of argument expansion in
ix86_expand_builtin function, including how Pmode is handled.

Also, please see how target is handled there. Target can be null, so
REG_P predicate will crash.

You should also select insn patterns depending on BNDmode, not TARGET_64BIT.

Please use assign_386_stack_local so stack slots can be shared.
SLOT_TEMP is intended for short-lived temporaries, you can introduce
new slots if you need more live values at once.

Uros.



More information about the Gcc-patches mailing list