* [PATCH] ARM: fix -masm-syntax-unified (PR88648)
@ 2019-01-01 23:34 Stefan Agner
2019-01-08 9:33 ` Kyrill Tkachov
0 siblings, 1 reply; 6+ messages in thread
From: Stefan Agner @ 2019-01-01 23:34 UTC (permalink / raw)
To: nickc, richard.earnshaw, ramana.radhakrishnan, kyrylo.tkachov
Cc: gcc-patches, Stefan Agner
This allows to use unified asm syntax when compiling for the
ARM instruction. This matches documentation and seems what the
initial patch was intended doing when the flag got added.
---
gcc/config/arm/arm.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 3419b6bd0f8..67b2b199f3f 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -3095,7 +3095,8 @@ arm_option_override_internal (struct gcc_options *opts,
/* Thumb2 inline assembly code should always use unified syntax.
This will apply to ARM and Thumb1 eventually. */
- opts->x_inline_asm_unified = TARGET_THUMB2_P (opts->x_target_flags);
+ if (TARGET_THUMB2_P (opts->x_target_flags))
+ opts->x_inline_asm_unified = true;
#ifdef SUBTARGET_OVERRIDE_INTERNAL_OPTIONS
SUBTARGET_OVERRIDE_INTERNAL_OPTIONS;
--
2.20.1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ARM: fix -masm-syntax-unified (PR88648)
2019-01-01 23:34 [PATCH] ARM: fix -masm-syntax-unified (PR88648) Stefan Agner
@ 2019-01-08 9:33 ` Kyrill Tkachov
2019-01-08 9:36 ` Kyrill Tkachov
2019-01-10 11:38 ` Kyrill Tkachov
0 siblings, 2 replies; 6+ messages in thread
From: Kyrill Tkachov @ 2019-01-08 9:33 UTC (permalink / raw)
To: Stefan Agner, nickc, Richard Earnshaw, Ramana Radhakrishnan; +Cc: gcc-patches
Hi Stefan,
On 01/01/19 23:34, Stefan Agner wrote:
> This allows to use unified asm syntax when compiling for the
> ARM instruction. This matches documentation and seems what the
> initial patch was intended doing when the flag got added.
> ---
> gcc/config/arm/arm.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> index 3419b6bd0f8..67b2b199f3f 100644
> --- a/gcc/config/arm/arm.c
> +++ b/gcc/config/arm/arm.c
> @@ -3095,7 +3095,8 @@ arm_option_override_internal (struct gcc_options *opts,
>
> /* Thumb2 inline assembly code should always use unified syntax.
> This will apply to ARM and Thumb1 eventually. */
> - opts->x_inline_asm_unified = TARGET_THUMB2_P (opts->x_target_flags);
> + if (TARGET_THUMB2_P (opts->x_target_flags))
> + opts->x_inline_asm_unified = true;
This looks right to me and is the logic we had in GCC 5.
How has this patch been tested?
Can you please provide a ChangeLog entry for this patch[1].
Thanks,
Kyrill
[1] https://gcc.gnu.org/contribute.html
>
> #ifdef SUBTARGET_OVERRIDE_INTERNAL_OPTIONS
> SUBTARGET_OVERRIDE_INTERNAL_OPTIONS;
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ARM: fix -masm-syntax-unified (PR88648)
2019-01-08 9:33 ` Kyrill Tkachov
@ 2019-01-08 9:36 ` Kyrill Tkachov
2019-01-10 11:38 ` Kyrill Tkachov
1 sibling, 0 replies; 6+ messages in thread
From: Kyrill Tkachov @ 2019-01-08 9:36 UTC (permalink / raw)
To: Stefan Agner, nickc, Richard Earnshaw, Ramana Radhakrishnan; +Cc: gcc-patches
On 08/01/19 09:33, Kyrill Tkachov wrote:
> Hi Stefan,
>
> On 01/01/19 23:34, Stefan Agner wrote:
> > This allows to use unified asm syntax when compiling for the
> > ARM instruction. This matches documentation and seems what the
> > initial patch was intended doing when the flag got added.
> > ---
> > gcc/config/arm/arm.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> > index 3419b6bd0f8..67b2b199f3f 100644
> > --- a/gcc/config/arm/arm.c
> > +++ b/gcc/config/arm/arm.c
> > @@ -3095,7 +3095,8 @@ arm_option_override_internal (struct gcc_options *opts,
> >
> > /* Thumb2 inline assembly code should always use unified syntax.
> > This will apply to ARM and Thumb1 eventually. */
> > - opts->x_inline_asm_unified = TARGET_THUMB2_P (opts->x_target_flags);
> > + if (TARGET_THUMB2_P (opts->x_target_flags))
> > + opts->x_inline_asm_unified = true;
>
> This looks right to me and is the logic we had in GCC 5.
> How has this patch been tested?
>
For the avoidance of doubt, I mean that your patch is correct :)
(not that the existing code is right).
> Can you please provide a ChangeLog entry for this patch[1].
>
> Thanks,
> Kyrill
>
> [1] https://gcc.gnu.org/contribute.html
>
> >
> > #ifdef SUBTARGET_OVERRIDE_INTERNAL_OPTIONS
> > SUBTARGET_OVERRIDE_INTERNAL_OPTIONS;
> > --
> > 2.20.1
> >
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ARM: fix -masm-syntax-unified (PR88648)
2019-01-08 9:33 ` Kyrill Tkachov
2019-01-08 9:36 ` Kyrill Tkachov
@ 2019-01-10 11:38 ` Kyrill Tkachov
2019-02-09 16:25 ` Stefan Agner
1 sibling, 1 reply; 6+ messages in thread
From: Kyrill Tkachov @ 2019-01-10 11:38 UTC (permalink / raw)
To: Stefan Agner, nickc, Richard Earnshaw, Ramana Radhakrishnan; +Cc: gcc-patches
Hi Stefan,
On 08/01/19 09:33, Kyrill Tkachov wrote:
> Hi Stefan,
>
> On 01/01/19 23:34, Stefan Agner wrote:
> > This allows to use unified asm syntax when compiling for the
> > ARM instruction. This matches documentation and seems what the
> > initial patch was intended doing when the flag got added.
> > ---
> > gcc/config/arm/arm.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> > index 3419b6bd0f8..67b2b199f3f 100644
> > --- a/gcc/config/arm/arm.c
> > +++ b/gcc/config/arm/arm.c
> > @@ -3095,7 +3095,8 @@ arm_option_override_internal (struct gcc_options *opts,
> >
> > /* Thumb2 inline assembly code should always use unified syntax.
> > This will apply to ARM and Thumb1 eventually. */
> > - opts->x_inline_asm_unified = TARGET_THUMB2_P (opts->x_target_flags);
> > + if (TARGET_THUMB2_P (opts->x_target_flags))
> > + opts->x_inline_asm_unified = true;
>
> This looks right to me and is the logic we had in GCC 5.
> How has this patch been tested?
>
> Can you please provide a ChangeLog entry for this patch[1].
>
I've bootstrapped and tested this, together with your testsuite patch on arm-none-linux-gnueabihf
and committed both with r267804 with the following ChangeLog entries:
2019-01-10 Stefan Agner <stefan@agner.ch>
PR target/88648
* config/arm/arm.c (arm_option_override_internal): Force
opts->x_inline_asm_unified to true only if TARGET_THUMB2_P.
2019-01-10 Stefan Agner <stefan@agner.ch>
PR target/88648
* gcc.target/arm/pr88648-asm-syntax-unified.c: Add test to
check if -masm-syntax-unified gets applied properly.
Thank you for the patch. If you plan to contribute more patches in the future I suggest you
sort out the copyright assignment paperwork.
I believe this fix needs to be backported to the branches.
I'll do so after a few days of testing on trunk.
Thanks again,
Kyrill
> Thanks,
> Kyrill
>
> [1] https://gcc.gnu.org/contribute.html
>
> >
> > #ifdef SUBTARGET_OVERRIDE_INTERNAL_OPTIONS
> > SUBTARGET_OVERRIDE_INTERNAL_OPTIONS;
> > --
> > 2.20.1
> >
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ARM: fix -masm-syntax-unified (PR88648)
2019-01-10 11:38 ` Kyrill Tkachov
@ 2019-02-09 16:25 ` Stefan Agner
2019-02-11 9:27 ` Kyrill Tkachov
0 siblings, 1 reply; 6+ messages in thread
From: Stefan Agner @ 2019-02-09 16:25 UTC (permalink / raw)
To: Kyrill Tkachov; +Cc: nickc, Richard Earnshaw, Ramana Radhakrishnan, gcc-patches
Hi Kyrill,
On 10.01.2019 12:38, Kyrill Tkachov wrote:
> Hi Stefan,
>
> On 08/01/19 09:33, Kyrill Tkachov wrote:
>> Hi Stefan,
>>
>> On 01/01/19 23:34, Stefan Agner wrote:
>> > This allows to use unified asm syntax when compiling for the
>> > ARM instruction. This matches documentation and seems what the
>> > initial patch was intended doing when the flag got added.
>> > ---
>> > gcc/config/arm/arm.c | 3 ++-
>> > 1 file changed, 2 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
>> > index 3419b6bd0f8..67b2b199f3f 100644
>> > --- a/gcc/config/arm/arm.c
>> > +++ b/gcc/config/arm/arm.c
>> > @@ -3095,7 +3095,8 @@ arm_option_override_internal (struct gcc_options *opts,
>> >
>> > /* Thumb2 inline assembly code should always use unified syntax.
>> > This will apply to ARM and Thumb1 eventually. */
>> > - opts->x_inline_asm_unified = TARGET_THUMB2_P (opts->x_target_flags);
>> > + if (TARGET_THUMB2_P (opts->x_target_flags))
>> > + opts->x_inline_asm_unified = true;
>>
>> This looks right to me and is the logic we had in GCC 5.
>> How has this patch been tested?
>>
>> Can you please provide a ChangeLog entry for this patch[1].
>>
>
> I've bootstrapped and tested this, together with your testsuite patch
> on arm-none-linux-gnueabihf
> and committed both with r267804 with the following ChangeLog entries:
>
> 2019-01-10 Stefan Agner <stefan@agner.ch>
>
> PR target/88648
> * config/arm/arm.c (arm_option_override_internal): Force
> opts->x_inline_asm_unified to true only if TARGET_THUMB2_P.
>
> 2019-01-10 Stefan Agner <stefan@agner.ch>
>
> PR target/88648
> * gcc.target/arm/pr88648-asm-syntax-unified.c: Add test to
> check if -masm-syntax-unified gets applied properly.
>
> Thank you for the patch. If you plan to contribute more patches in the
> future I suggest you
> sort out the copyright assignment paperwork.
>
> I believe this fix needs to be backported to the branches.
> I'll do so after a few days of testing on trunk.
Thanks for applying the patch! As far as I can see it did not made it
into the branch yet, do you think it can get backported there too?
--
Stefan
>
> Thanks again,
> Kyrill
>
>> Thanks,
>> Kyrill
>>
>> [1] https://gcc.gnu.org/contribute.html
>>
>> >
>> > #ifdef SUBTARGET_OVERRIDE_INTERNAL_OPTIONS
>> > SUBTARGET_OVERRIDE_INTERNAL_OPTIONS;
>> > --
>> > 2.20.1
>> >
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ARM: fix -masm-syntax-unified (PR88648)
2019-02-09 16:25 ` Stefan Agner
@ 2019-02-11 9:27 ` Kyrill Tkachov
0 siblings, 0 replies; 6+ messages in thread
From: Kyrill Tkachov @ 2019-02-11 9:27 UTC (permalink / raw)
To: Stefan Agner; +Cc: nickc, Richard Earnshaw, Ramana Radhakrishnan, gcc-patches
Hi Stefan,
On 09/02/19 16:25, Stefan Agner wrote:
> Hi Kyrill,
>
> On 10.01.2019 12:38, Kyrill Tkachov wrote:
>> Hi Stefan,
>>
>> On 08/01/19 09:33, Kyrill Tkachov wrote:
>>> Hi Stefan,
>>>
>>> On 01/01/19 23:34, Stefan Agner wrote:
>>>> This allows to use unified asm syntax when compiling for the
>>>> ARM instruction. This matches documentation and seems what the
>>>> initial patch was intended doing when the flag got added.
>>>> ---
>>>> gcc/config/arm/arm.c | 3 ++-
>>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
>>>> index 3419b6bd0f8..67b2b199f3f 100644
>>>> --- a/gcc/config/arm/arm.c
>>>> +++ b/gcc/config/arm/arm.c
>>>> @@ -3095,7 +3095,8 @@ arm_option_override_internal (struct gcc_options *opts,
>>>>
>>>> /* Thumb2 inline assembly code should always use unified syntax.
>>>> This will apply to ARM and Thumb1 eventually. */
>>>> - opts->x_inline_asm_unified = TARGET_THUMB2_P (opts->x_target_flags);
>>>> + if (TARGET_THUMB2_P (opts->x_target_flags))
>>>> + opts->x_inline_asm_unified = true;
>>> This looks right to me and is the logic we had in GCC 5.
>>> How has this patch been tested?
>>>
>>> Can you please provide a ChangeLog entry for this patch[1].
>>>
>> I've bootstrapped and tested this, together with your testsuite patch
>> on arm-none-linux-gnueabihf
>> and committed both with r267804 with the following ChangeLog entries:
>>
>> 2019-01-10 Stefan Agner <stefan@agner.ch>
>>
>> PR target/88648
>> * config/arm/arm.c (arm_option_override_internal): Force
>> opts->x_inline_asm_unified to true only if TARGET_THUMB2_P.
>>
>> 2019-01-10 Stefan Agner <stefan@agner.ch>
>>
>> PR target/88648
>> * gcc.target/arm/pr88648-asm-syntax-unified.c: Add test to
>> check if -masm-syntax-unified gets applied properly.
>>
>> Thank you for the patch. If you plan to contribute more patches in the
>> future I suggest you
>> sort out the copyright assignment paperwork.
>>
>> I believe this fix needs to be backported to the branches.
>> I'll do so after a few days of testing on trunk.
> Thanks for applying the patch! As far as I can see it did not made it
> into the branch yet, do you think it can get backported there too?
Thanks for reminding me. I've now committed your patch and the test to the gcc-8 branch with r268764.
Kyrill
> --
> Stefan
>
>> Thanks again,
>> Kyrill
>>
>>> Thanks,
>>> Kyrill
>>>
>>> [1] https://gcc.gnu.org/contribute.html
>>>
>>>> #ifdef SUBTARGET_OVERRIDE_INTERNAL_OPTIONS
>>>> SUBTARGET_OVERRIDE_INTERNAL_OPTIONS;
>>>> --
>>>> 2.20.1
>>>>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-02-11 9:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-01 23:34 [PATCH] ARM: fix -masm-syntax-unified (PR88648) Stefan Agner
2019-01-08 9:33 ` Kyrill Tkachov
2019-01-08 9:36 ` Kyrill Tkachov
2019-01-10 11:38 ` Kyrill Tkachov
2019-02-09 16:25 ` Stefan Agner
2019-02-11 9:27 ` Kyrill Tkachov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).