Hi Kyrill,
On 19/11/2019 11:21, Kyrill Tkachov wrote:
> Hi Dennis,
>
> On 11/12/19 5:32 PM, Dennis Zhang wrote:
>> Hi Kyrill,
>>
>> On 12/11/2019 15:57, Kyrill Tkachov wrote:
>>> On 11/12/19 3:50 PM, Dennis Zhang wrote:
>>>> Hi Kyrill,
>>>>
>>>> On 12/11/2019 09:40, Kyrill Tkachov wrote:
>>>>> Hi Dennis,
>>>>>
>>>>> On 11/7/19 1:48 PM, Dennis Zhang wrote:
>>>>>> Hi Kyrill,
>>>>>>
>>>>>> I have rebased the patch on top of current truck.
>>>>>> For resolve_overloaded, I redefined my memtag overloading function to
>>>>>> fit the latest resolve_overloaded_builtin interface.
>>>>>>
>>>>>> Regression tested again and survived for aarch64-none-linux-gnu.
>>>>> Please reply inline rather than top-posting on gcc-patches.
>>>>>
>>>>>
>>>>>> Cheers
>>>>>> Dennis
>>>>>>
>>>>>> Changelog is updated as following:
>>>>>>
>>>>>> gcc/ChangeLog:
>>>>>>
>>>>>> 2019-11-07 Dennis Zhang <dennis.zh...@arm.com>
>>>>>>
>>>>>> * config/aarch64/aarch64-builtins.c (enum aarch64_builtins):
>>>>>> Add
>>>>>> AARCH64_MEMTAG_BUILTIN_START, AARCH64_MEMTAG_BUILTIN_IRG,
>>>>>> AARCH64_MEMTAG_BUILTIN_GMI, AARCH64_MEMTAG_BUILTIN_SUBP,
>>>>>> AARCH64_MEMTAG_BUILTIN_INC_TAG, AARCH64_MEMTAG_BUILTIN_SET_TAG,
>>>>>> AARCH64_MEMTAG_BUILTIN_GET_TAG, and AARCH64_MEMTAG_BUILTIN_END.
>>>>>> (aarch64_init_memtag_builtins): New.
>>>>>> (AARCH64_INIT_MEMTAG_BUILTINS_DECL): New macro.
>>>>>> (aarch64_general_init_builtins): Call
>>>>>> aarch64_init_memtag_builtins.
>>>>>> (aarch64_expand_builtin_memtag): New.
>>>>>> (aarch64_general_expand_builtin): Call
>>>>>> aarch64_expand_builtin_memtag.
>>>>>> (AARCH64_BUILTIN_SUBCODE): New macro.
>>>>>> (aarch64_resolve_overloaded_memtag): New.
>>>>>> (aarch64_resolve_overloaded_builtin_general): New hook. Call
>>>>>> aarch64_resolve_overloaded_memtag to handle overloaded MTE
>>>>>> builtins.
>>>>>> * config/aarch64/aarch64-c.c (aarch64_update_cpp_builtins):
>>>>>> Define
>>>>>> __ARM_FEATURE_MEMORY_TAGGING when enabled.
>>>>>> (aarch64_resolve_overloaded_builtin): Call
>>>>>> aarch64_resolve_overloaded_builtin_general.
>>>>>> * config/aarch64/aarch64-protos.h
>>>>>> (aarch64_resolve_overloaded_builtin_general): New declaration.
>>>>>> * config/aarch64/aarch64.h (AARCH64_ISA_MEMTAG): New macro.
>>>>>> (TARGET_MEMTAG): Likewise.
>>>>>> * config/aarch64/aarch64.md (define_c_enum "unspec"): Add
>>>>>> UNSPEC_GEN_TAG, UNSPEC_GEN_TAG_RND, and UNSPEC_TAG_SPACE.
>>>>>> (irg, gmi, subp, addg, ldg, stg): New instructions.
>>>>>> * config/aarch64/arm_acle.h (__arm_mte_create_random_tag): New
>>>>>> macro.
>>>>>> (__arm_mte_exclude_tag, __arm_mte_increment_tag): Likewise.
>>>>>> (__arm_mte_ptrdiff, __arm_mte_set_tag, __arm_mte_get_tag):
>>>>>> Likewise.
>>>>>> * config/aarch64/predicates.md (aarch64_memtag_tag_offset):
>>>>>> New.
>>>>>> (aarch64_granule16_uimm6, aarch64_granule16_simm9): New.
>>>>>> * config/arm/types.md (memtag): New.
>>>>>> * doc/invoke.texi (-memtag): Update description.
>>>>>>
>>>>>> gcc/testsuite/ChangeLog:
>>>>>>
>>>>>> 2019-11-07 Dennis Zhang <dennis.zh...@arm.com>
>>>>>>
>>>>>> * gcc.target/aarch64/acle/memtag_1.c: New test.
>>>>>> * gcc.target/aarch64/acle/memtag_2.c: New test.
>>>>>> * gcc.target/aarch64/acle/memtag_3.c: New test.
>>>>>>
>>>>>>
>>>>>> On 04/11/2019 16:40, Kyrill Tkachov wrote:
>>>>>>> Hi Dennis,
>>>>>>>
>>>>>>> On 10/17/19 11:03 AM, Dennis Zhang wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> Arm Memory Tagging Extension (MTE) is published with Armv8.5-A.
>>>>>>>> It can be used for spatial and temporal memory safety detection and
>>>>>>>> lightweight lock and key system.
>>>>>>>>
>>>>>>>> This patch enables new intrinsics leveraging MTE instructions to
>>>>>>>> implement functionalities of creating tags, setting tags, reading
>>>>>>>> tags,
>>>>>>>> and manipulating tags.
>>>>>>>> The intrinsics are part of Arm ACLE extension:
>>>>>>>> https://developer.arm.com/docs/101028/latest/memory-tagging-intrinsics
>>>>>>>>
>>>>>>>>
>>>>>>>> The MTE ISA specification can be found at
>>>>>>>> https://developer.arm.com/docs/ddi0487/latest chapter D6.
>>>>>>>>
>>>>>>>> Bootstraped and regtested for aarch64-none-linux-gnu.
>>>>>>>>
>>>>>>>> Please help to check if it's OK for trunk.
>>>>>>>>
>>>>>>> This looks mostly ok to me but for further review this needs to be
>>>>>>> rebased on top of current trunk as there are some conflicts with
>>>>>>> the SVE
>>>>>>> ACLE changes that recently went in. Most conflicts looks trivial to
>>>>>>> resolve but one that needs more attention is the definition of the
>>>>>>> TARGET_RESOLVE_OVERLOADED_BUILTIN hook.
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Kyrill
>>>>>>>
>>>>>>>> Many Thanks
>>>>>>>> Dennis
>>>>>>>>
>>>>>>>> gcc/ChangeLog:
>>>>>>>>
>>>>>>>> 2019-10-16 Dennis Zhang <dennis.zh...@arm.com>
>>>>>>>>
>>>>>>>> * config/aarch64/aarch64-builtins.c (enum
>>>>>>>> aarch64_builtins): Add
>>>>>>>> AARCH64_MEMTAG_BUILTIN_START,
>>>>>>>> AARCH64_MEMTAG_BUILTIN_IRG,
>>>>>>>> AARCH64_MEMTAG_BUILTIN_GMI, AARCH64_MEMTAG_BUILTIN_SUBP,
>>>>>>>> AARCH64_MEMTAG_BUILTIN_INC_TAG,
>>>>>>>> AARCH64_MEMTAG_BUILTIN_SET_TAG,
>>>>>>>> AARCH64_MEMTAG_BUILTIN_GET_TAG, and
>>>>>>>> AARCH64_MEMTAG_BUILTIN_END.
>>>>>>>> (aarch64_init_memtag_builtins): New.
>>>>>>>> (AARCH64_INIT_MEMTAG_BUILTINS_DECL): New macro.
>>>>>>>> (aarch64_general_init_builtins): Call
>>>>>>>> aarch64_init_memtag_builtins.
>>>>>>>> (aarch64_expand_builtin_memtag): New.
>>>>>>>> (aarch64_general_expand_builtin): Call
>>>>>>>> aarch64_expand_builtin_memtag.
>>>>>>>> (AARCH64_BUILTIN_SUBCODE): New macro.
>>>>>>>> (aarch64_resolve_overloaded_memtag): New.
>>>>>>>> (aarch64_resolve_overloaded_builtin): New hook. Call
>>>>>>>> aarch64_resolve_overloaded_memtag to handle
>>>>>>>> overloaded MTE
>>>>>>>> builtins.
>>>>>>>> * config/aarch64/aarch64-c.c
>>>>>>>> (aarch64_update_cpp_builtins):
>>>>>>>> Define
>>>>>>>> __ARM_FEATURE_MEMORY_TAGGING when enabled.
>>>>>>>> * config/aarch64/aarch64-protos.h
>>>>>>>> (aarch64_resolve_overloaded_builtin):
>>>>>>>> Add declaration.
>>>>>>>> * config/aarch64/aarch64.c
>>>>>>>> (TARGET_RESOLVE_OVERLOADED_BUILTIN):
>>>>>>>> New hook.
>>>>>>>> * config/aarch64/aarch64.h (AARCH64_ISA_MEMTAG): New
>>>>>>>> macro.
>>>>>>>> (TARGET_MEMTAG): Likewise.
>>>>>>>> * config/aarch64/aarch64.md (define_c_enum "unspec"):
>>>>>>>> Add
>>>>>>>> UNSPEC_GEN_TAG, UNSPEC_GEN_TAG_RND, and
>>>>>>>> UNSPEC_TAG_SPACE.
>>>>>>>> (irg, gmi, subp, addg, ldg, stg): New instructions.
>>>>>>>> * config/aarch64/arm_acle.h
>>>>>>>> (__arm_mte_create_random_tag): New
>>>>>>>> macro.
>>>>>>>> (__arm_mte_exclude_tag, __arm_mte_increment_tag):
>>>>>>>> Likewise.
>>>>>>>> (__arm_mte_ptrdiff, __arm_mte_set_tag,
>>>>>>>> __arm_mte_get_tag):
>>>>>>>> Likewise.
>>>>>>>> * config/aarch64/predicates.md
>>>>>>>> (aarch64_memtag_tag_offset):
>>>>>>>> New.
>>>>>>>> (aarch64_granule16_uimm6, aarch64_granule16_simm9): New.
>>>>>>>> * config/arm/types.md (memtag): New.
>>>>>>>> * doc/invoke.texi (-memtag): Update description.
>>>>>>>>
>>>>>>>> gcc/testsuite/ChangeLog:
>>>>>>>>
>>>>>>>> 2019-10-16 Dennis Zhang <dennis.zh...@arm.com>
>>>>>>>>
>>>>>>>> * gcc.target/aarch64/acle/memtag_1.c: New test.
>>>>>>>> * gcc.target/aarch64/acle/memtag_2.c: New test.
>>>>>>>> * gcc.target/aarch64/acle/memtag_3.c: New test.
>>>>> +/* Expand an expression EXP that calls a MEMTAG built-in FCODE
>>>>> + with result going to TARGET. */
>>>>> +static rtx
>>>>> +aarch64_expand_builtin_memtag (int fcode, tree exp, rtx target)
>>>>> +{
>>>>> + rtx pat = NULL;
>>>>> + enum insn_code icode = aarch64_memtag_builtin_data[fcode -
>>>>> + AARCH64_MEMTAG_BUILTIN_START - 1].icode;
>>>>> +
>>>>> + rtx op0 = expand_normal (CALL_EXPR_ARG (exp, 0));
>>>>> + machine_mode mode0 = GET_MODE (op0);
>>>>> + op0 = force_reg (mode0 == VOIDmode ? DImode : mode0, op0);
>>>>> + op0 = convert_to_mode (DImode, op0, true);
>>>>> +
>>>>> + switch (fcode)
>>>>> + {
>>>>> + case AARCH64_MEMTAG_BUILTIN_IRG:
>>>>> + case AARCH64_MEMTAG_BUILTIN_GMI:
>>>>> + case AARCH64_MEMTAG_BUILTIN_SUBP:
>>>>> + case AARCH64_MEMTAG_BUILTIN_INC_TAG:
>>>>> + {
>>>>> + if (! target
>>>>> + || GET_MODE (target) != DImode
>>>>> + || ! (*insn_data[icode].operand[0].predicate) (target,
>>>>> DImode))
>>>>> + target = gen_reg_rtx (DImode);
>>>>> +
>>>>> + if (fcode == AARCH64_MEMTAG_BUILTIN_INC_TAG)
>>>>> + {
>>>>> + rtx op1 = expand_normal (CALL_EXPR_ARG (exp, 1));
>>>>> +
>>>>> + if ((*insn_data[icode].operand[3].predicate) (op1, QImode))
>>>>> + {
>>>>> + pat = GEN_FCN (icode) (target, op0, const0_rtx, op1);
>>>>> + break;
>>>>> + }
>>>>> + error ("%Kargument %d must be a constant immediate "
>>>>> + "in range [0,15]", exp, 2);
>>>>> + return target;
>>>>> + }
>>>>> + else
>>>>> + {
>>>>> + rtx op1 = expand_normal (CALL_EXPR_ARG (exp, 1));
>>>>> + machine_mode mode1 = GET_MODE (op1);
>>>>> + op1 = force_reg (mode1 == VOIDmode ? DImode : mode1, op1);
>>>>> + op1 = convert_to_mode (DImode, op1, true);
>>>>> + pat = GEN_FCN (icode) (target, op0, op1);
>>>>> + }
>>>>> + break;
>>>>> + }
>>>>> + case AARCH64_MEMTAG_BUILTIN_GET_TAG:
>>>>> + target = op0;
>>>>> + pat = GEN_FCN (icode) (target, op0, const0_rtx);
>>>>> + break;
>>>>> + case AARCH64_MEMTAG_BUILTIN_SET_TAG:
>>>>> + pat = GEN_FCN (icode) (op0, op0, const0_rtx);
>>>>> + break;
>>>>> + default:
>>>>> + gcc_unreachable();
>>>>> + }
>>>>> +
>>>>> + if (!pat)
>>>>> + return NULL_RTX;
>>>>> +
>>>>> + emit_insn (pat);
>>>>> + return target;
>>>>> +}
>>>>> +
>>>>>
>>>>> I think we want to error out when using -mabi=ilp32.
>>>>> Can you please add an error message when they are used with
>>>>> TARGET_ILP32.
>>>> Done. And it's tested in testsuite memtag_3.c.
>>>>
>>>>> diff --git a/gcc/testsuite/gcc.target/aarch64/acle/memtag_1.c
>>>>> b/gcc/testsuite/gcc.target/aarch64/acle/memtag_1.c
>>>>> new file mode 100644
>>>>> index 00000000000..a738f7e0b58
>>>>> --- /dev/null
>>>>> +++ b/gcc/testsuite/gcc.target/aarch64/acle/memtag_1.c
>>>>> @@ -0,0 +1,61 @@
>>>>> +/* Test the MEMTAG ACLE intrinsic. */
>>>>> +
>>>>> +/* { dg-do compile { target aarch64-*-* } } */
>>>>> +/* { dg-options "-O2 -march=armv8.5-a+memtag" } */
>>>>> +
>>>>>
>>>>> You shouldn't need the { target aarch64-*-* } here unless you're
>>>>> trying
>>>>> to restrict this for big-endian?
>>>>> Instead we should gate these on lp64 target to prevent these from
>>>>> running on ILP32 targets.
>>>> Fixed. And regtested again for aarch64-none-linux-gnu.
>>>> Thanks a lot for the feedback.
>>>>
>>>>> Ok with those two changes.
>>>>> Thanks for the patch!
>>>>> Kyrill
>>>>>
>>>>> +#include "arm_acle.h"
>>>>> +
>>> +/* Expand an expression EXP that calls a MEMTAG built-in FCODE
>>> + with result going to TARGET. */
>>> +static rtx
>>> +aarch64_expand_builtin_memtag (int fcode, tree exp, rtx target)
>>> +{
>>> + if (TARGET_ILP32)
>>> + error ("Memory Tagging Extension does not support '-mabi=ilp32'");
>>>
>>>
>>> You have to return const0_rtx here (see the other calls to error () in
>>> this file) rather than continue with the processing.
>>> Also, to render the quotes properly use %<-mabi=ilp32%> in the string.
>>>
>> The patch is fixed as attached.
>
> This patch is ok.
>
> Last thing to do, please provide an updated ChangeLog entry and I'll get
> this committed for you.
The ChangeLog is updated as following.
Many Thanks!
Dennis
gcc/ChangeLog:
2019-11-19 Dennis Zhang <dennis.zh...@arm.com>
* config/aarch64/aarch64-builtins.c (enum aarch64_builtins): Add
AARCH64_MEMTAG_BUILTIN_START, AARCH64_MEMTAG_BUILTIN_IRG,
AARCH64_MEMTAG_BUILTIN_GMI, AARCH64_MEMTAG_BUILTIN_SUBP,
AARCH64_MEMTAG_BUILTIN_INC_TAG, AARCH64_MEMTAG_BUILTIN_SET_TAG,
AARCH64_MEMTAG_BUILTIN_GET_TAG, and AARCH64_MEMTAG_BUILTIN_END.
(aarch64_init_memtag_builtins): New.
(AARCH64_INIT_MEMTAG_BUILTINS_DECL): New macro.
(aarch64_general_init_builtins): Call aarch64_init_memtag_builtins.
(aarch64_expand_builtin_memtag): New.
(aarch64_general_expand_builtin): Call aarch64_expand_builtin_memtag.
(AARCH64_BUILTIN_SUBCODE): New macro.
(aarch64_resolve_overloaded_memtag): New.
(aarch64_resolve_overloaded_builtin_general): New. Call
aarch64_resolve_overloaded_memtag to handle overloaded MTE builtins.
* config/aarch64/aarch64-c.c (aarch64_update_cpp_builtins): Define
__ARM_FEATURE_MEMORY_TAGGING when enabled.
(aarch64_resolve_overloaded_builtin): Call
aarch64_resolve_overloaded_builtin_general.
* config/aarch64/aarch64-protos.h
(aarch64_resolve_overloaded_builtin_general): New declaration.
* config/aarch64/aarch64.h (AARCH64_ISA_MEMTAG): New macro.
(TARGET_MEMTAG): Likewise.
* config/aarch64/aarch64.md (UNSPEC_GEN_TAG): New unspec.
(UNSPEC_GEN_TAG_RND, and UNSPEC_TAG_SPACE): Likewise.
(irg, gmi, subp, addg, ldg, stg): New instructions.
* config/aarch64/arm_acle.h (__arm_mte_create_random_tag): New macro.
(__arm_mte_exclude_tag, __arm_mte_ptrdiff): Likewise.
(__arm_mte_increment_tag, __arm_mte_set_tag): Likewise.
(__arm_mte_get_tag): Likewise.
* config/aarch64/predicates.md (aarch64_memtag_tag_offset): New.
(aarch64_granule16_uimm6, aarch64_granule16_simm9): New.
* config/arm/types.md (memtag): New.
* doc/invoke.texi (-memtag): Update description.
gcc/testsuite/ChangeLog:
2019-11-19 Dennis Zhang <dennis.zh...@arm.com>
* gcc.target/aarch64/acle/memtag_1.c: New test.
* gcc.target/aarch64/acle/memtag_2.c: New test.
* gcc.target/aarch64/acle/memtag_3.c: New test.
