Sorry for the late reply.
Indu Bhagat <indu.bha...@oracle.com> writes:
> On 4/22/25 11:00 AM, Indu Bhagat wrote:
>> On 4/15/25 8:35 AM, Richard Sandiford wrote:
>>> Hi,
>>>
>>> Indu Bhagat <indu.bha...@oracle.com> writes:
>>>> subg (Subtract with Tag) is an Armv8.5-A memory tagging (MTE)
>>>> instruction. It can be used to subtract an immediate value scaled by
>>>> the tag granule from the address in the source register.
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>> * config/aarch64/aarch64.md (subg): New definition.
>>>
>>> In my previous comment about this patch:
>>>
>>> https://gcc.gnu.org/pipermail/gcc-patches/2024-November/668669.html
>>>
>>> I hadn't realised that the pattern follows the existing "addg" pattern.
>>> But...
>>>
>>
>> And I just realized that it seems some of my edits to this commit got
>> lost in a rebase or such in my work branch.
>>
>> I had addressed the review comments earlier on this patch, but they
>> didnt make it out in RFC V2.
>>
>>>> ---
>>>> gcc/config/aarch64/aarch64.md | 17 +++++++++++++++++
>>>> 1 file changed, 17 insertions(+)
>>>>
>>>> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/
>>>> aarch64.md
>>>> index 031e621c98a1..0c7aebb838cd 100644
>>>> --- a/gcc/config/aarch64/aarch64.md
>>>> +++ b/gcc/config/aarch64/aarch64.md
>>>> @@ -8416,6 +8416,23 @@
>>>> [(set_attr "type" "memtag")]
>>>> )
>>>> +(define_insn "subg"
>>>> + [(set (match_operand:DI 0 "register_operand" "=rk")
>>>> + (ior:DI
>>>> + (and:DI (minus:DI (match_operand:DI 1 "register_operand" "rk")
>>>> + (match_operand:DI 2 "aarch64_granule16_uimm6" "i"))
>>>> + (const_int -1080863910568919041)) ;; 0xf0ff...
>>>> + (ashift:DI
>>>> + (unspec:QI
>>>> + [(and:QI (lshiftrt:DI (match_dup 1) (const_int 56))
>>>> (const_int 15))
>>>> + (match_operand:QI 3 "aarch64_memtag_tag_offset" "i")]
>>>> + UNSPEC_GEN_TAG)
>>>> + (const_int 56))))]
>>>> + "TARGET_MEMTAG"
>>>> + "subg\\t%0, %1, #%2, #%3"
>>>> + [(set_attr "type" "memtag")]
>>>> +)
>>>> +
>>>> (define_insn "subp"
>>>> [(set (match_operand:DI 0 "register_operand" "=r")
>>>> (minus:DI
>>>
>>> ...subtractions of constants are canonically expressed using (plus ...)
>>> of a negative number, rather than (minus ...) of a positive number.
>>> So I think we should instead add subg to the existing addg pattern.
>>> That is, in:
>>>
>>> (define_insn "addg"
>>> [(set (match_operand:DI 0 "register_operand" "=rk")
>>> (ior:DI
>>> (and:DI (plus:DI (match_operand:DI 1 "register_operand" "rk")
>>> (match_operand:DI 2 "aarch64_granule16_uimm6" "i"))
>>> (const_int -1080863910568919041)) ;; 0xf0ff...
>>> (ashift:DI
>>> (unspec:QI
>>> [(and:QI (lshiftrt:DI (match_dup 1) (const_int 56)) (const_int
>>> 15))
>>> (match_operand:QI 3 "aarch64_memtag_tag_offset" "i")]
>>> UNSPEC_GEN_TAG)
>>> (const_int 56))))]
>>> "TARGET_MEMTAG"
>>> "addg\\t%0, %1, #%2, #%3"
>>> [(set_attr "type" "memtag")]
>>> )
>>>
>>> the aarch64_granule16_uimm6 would be replaced with a predicate that
>>> accepts all multiples of 16 in the range [-1008, 1008]. Then the
>>> output pattern would generate an addg or subg instruction based on
>>> whether operand 2 is negative.
>>>
>>
>> ;; These constants are used as a const_int in MTE instructions
>> (define_constants
>> [; 0xf0ff...
>> ; Tag mask for the 4-bit tag stored in the top 8 bits of a pointer.
>> (MEMTAG_TAG_MASK -1080863910568919041)])
>>
>> Above was requested by Andrew. If what is shown here (both defining the
>> constant and using it below) in the email OK to you, I can submit a
>> separate patch to then use this const in existing irg, ldg..
>>
>> with predicates.md saying:
>>
>> (define_predicate "aarch64_granule16_imm6"
>> (and (match_code "const_int")
>> (match_test "IN_RANGE (INTVAL (op), -1008, 1008)
The minimum can be -1020 instead of -1008.
>> && !(INTVAL (op) & 0xf)")))
>>
>> addg pattern as follows:
>>
>> (define_insn "addg"
>> [(set (match_operand:DI 0 "register_operand")
>> (ior:DI
>> (and:DI (plus:DI (match_operand:DI 1 "register_operand")
>> (match_operand:DI 2 "aarch64_granule16_imm6"))
>> (const_int MEMTAG_TAG_MASK))
>> (ashift:DI
>> (unspec:DI [(match_dup 1)
>> (match_operand:QI 3 "aarch64_memtag_tag_offset")]
>> UNSPEC_GEN_TAG)
>> (const_int 56))))]
>> "TARGET_MEMTAG"
>> {@ [ cons: =0 , 1 , 2 , 3 ; attrs: type ]
>> [ rk , rk , I , I ; memtag ] addg\t%0, %1, #%2, #%3
>> [ rk , rk , J , I ; memtag ] subg\t%0, %1, #%2, #%3
>> }
>> )
>>
>
> Sorry, it should be subg\t%0, %1, #%n2, #%3.
Yeah.
> Also I need to define new Ix and Jx constraints suiting imm6...
They would need to start with something other than "I" or "J",
since the number of characters in a constraint is determined
by the first character.
Thanks,
Richard
