Re: [PATCH, ARM 5/7] Add support for MOVT/MOVW to ARMv8-M Baseline

[Thomas Preudhomme]

On 10/10/16 13:35, Christophe Lyon wrote:

Hi Thomas,


Hi Christophe,



On 13 July 2016 at 17:34, Thomas Preudhomme
 wrote:

On Wednesday 13 July 2016 17:14:52 Christophe Lyon wrote:

Hi Thomas,


Hi Christophe,



I'm seeing:
gcc.target/arm/pr42574.c: syntax error in target selector
"arm_thumb1_ok && { ! arm_thumb1_movt_ok }" for " dg-do 1 compile {
arm_thumb1_ok && { ! arm_thumb1_movt_ok } } "


Oops. I remember the trial and error to find the right amount of curly braces
yet I can indeed reproduce the error now. The target keyword is missing. I'll
submit a patch asap.

Best regards,

Thomas


I've noticed that the new test
  gcc.target/arm/movdi_movw.c scan-assembler-times movw\tr0, #61680 1
fails on armeb-none-linux-gnueabihf
--with-mode=thumb --with-cpu=cortex-a9 --with-fpu=neon-fp16


It seems DImode values are stored differently in register depending on the 
endianness. For little endian the low bits would be in the lowest register, but 
it's reverse in big endian. I'll adapt the test accordingly.




the other new tests pass, and using --with=mode=arm makes all three
of them unsupported.


Right, there is no reason to forbid ARM state indeed. I'll fix that too.



Sorry I missed it when I reported the other error.


No worry.



Can you have a look?


Absolutely.

Cheers,

Thomas

Re: [PATCH, ARM 5/7] Add support for MOVT/MOVW to ARMv8-M Baseline

[Christophe Lyon]

Hi Thomas,

On 13 July 2016 at 17:34, Thomas Preudhomme
 wrote:
> On Wednesday 13 July 2016 17:14:52 Christophe Lyon wrote:
>> Hi Thomas,
>
> Hi Christophe,
>
>>
>> I'm seeing:
>> gcc.target/arm/pr42574.c: syntax error in target selector
>> "arm_thumb1_ok && { ! arm_thumb1_movt_ok }" for " dg-do 1 compile {
>> arm_thumb1_ok && { ! arm_thumb1_movt_ok } } "
>
> Oops. I remember the trial and error to find the right amount of curly braces
> yet I can indeed reproduce the error now. The target keyword is missing. I'll
> submit a patch asap.
>
> Best regards,
>
> Thomas

I've noticed that the new test
  gcc.target/arm/movdi_movw.c scan-assembler-times movw\tr0, #61680 1
fails on armeb-none-linux-gnueabihf
--with-mode=thumb --with-cpu=cortex-a9 --with-fpu=neon-fp16

the other new tests pass, and using --with=mode=arm makes all three
of them unsupported.

Sorry I missed it when I reported the other error.

Can you have a look?

Thanks

Christophe

Re: [PATCH, ARM 5/7] Add support for MOVT/MOVW to ARMv8-M Baseline

[Thomas Preudhomme]

On Wednesday 13 July 2016 17:14:52 Christophe Lyon wrote:
> Hi Thomas,

Hi Christophe,

> 
> I'm seeing:
> gcc.target/arm/pr42574.c: syntax error in target selector
> "arm_thumb1_ok && { ! arm_thumb1_movt_ok }" for " dg-do 1 compile {
> arm_thumb1_ok && { ! arm_thumb1_movt_ok } } "

Oops. I remember the trial and error to find the right amount of curly braces 
yet I can indeed reproduce the error now. The target keyword is missing. I'll 
submit a patch asap.

Best regards,

Thomas

Re: [PATCH, ARM 5/7] Add support for MOVT/MOVW to ARMv8-M Baseline

[Christophe Lyon]

Hi Thomas,


On 13 July 2016 at 12:01, Kyrill Tkachov  wrote:
> Hi Thomas,
>
>
> On 07/07/16 15:32, Thomas Preudhomme wrote:
>>
>> Hi Kyrill,
>>
>> Please find an updated version in attachment. Please note I made quite a
>> few
>> other changes as well. The most important one was to add new ARMv8-M
>> Baseline
>> only alternatives to the two movt insns in order to have non predicable
>> output
>> template for ARMv8-M Baseline. I also inverted the logic for how to detect
>> ARMv8-M Baseline in the testsuite (arm_thumb1_movt_ok rather than *_ko)
>> and
>> fixed a couple of typos. I didn't add MOVT test because it is difficult to
>> generate. Using -mslow-flash-data should help but it is not available for
>> ARMv8-M Baseline as only config/arm/thumb2.md was modified to support this
>> option.
>>
>> Update ChangeLog entries are as follow:
>>
>>
>> *** gcc/ChangeLog ***
>>
>> 2016-06-20  Thomas Preud'homme  
>>
>>  * config/arm/arm.h (TARGET_HAVE_MOVT): Include ARMv8-M as having
>> MOVT.
>>  * config/arm/arm.c (arm_arch_name): (const_ok_for_op): Check
>> MOVT/MOVW
>>  availability with TARGET_HAVE_MOVT.
>>  (thumb_legitimate_constant_p): Strip the high part of a
>> label_ref.
>>  (thumb1_rtx_costs): Also return 0 if setting a half word constant
>> and
>>  MOVW is available and replace (unsigned HOST_WIDE_INT) INTVAL by
>>  UINTVAL.
>>  (thumb1_size_rtx_costs): Make set of half word constant also cost
>> 1
>>  extra instruction if MOVW is available.  Use a cost variable
>>  incremented by COSTS_N_INSNS (1) when the condition match rather
>> than
>>  returning an arithmetic expression based on COSTS_N_INSNS.  Make
>>  constant with bottom half word zero cost 2 instruction if MOVW is
>>  available.
>>  * config/arm/arm.md (define_attr "arch"): Add v8mb.
>>  (define_attr "arch_enabled"): Set to yes if arch value is v8mb
>> and
>>  target is ARMv8-M Baseline.
>>  (arm_movt): New unpredicable alternative for ARMv8-M Baseline.
>>  (arm_movtas_ze): Likewise.
>>  * config/arm/thumb1.md (thumb1_movdi_insn): Add ARMv8-M Baseline
>> only
>>  alternative for constants satisfying j constraint.
>>  (thumb1_movsi_insn): Likewise.
>>  (movsi splitter for K alternative): Tighten condition to not
>> trigger
>>  if movt is available and j constraint is satisfied.
>>  (Pe immediate splitter): Likewise.
>>  (thumb1_movhi_insn): Add ARMv8-M Baseline only alternative for
>>  constant fitting in an halfword to use MOVW.
>>  * doc/sourcebuild.texi (arm_thumb1_movt_ok): Document new ARM
>>  effective target.
>>
>>
>> *** gcc/testsuite/ChangeLog ***
>>
>> 2016-05-23  Thomas Preud'homme  
>>
>>  * lib/target-supports.exp
>> (check_effective_target_arm_thumb1_movt_ok):
>>  Define effective target.
>>  * gcc.target/arm/pr42574.c: Require arm_thumb1_ok and
>>  !arm_thumb1_movt_ok to exclude ARMv8-M Baseline.
>>  * gcc.target/arm/movhi_movw.c: New test.
>>  * gcc.target/arm/movsi_movw.c: Likewise.
>>  * gcc.target/arm/movdi_movw.c: Likewise.
>
>
> Ok.
> Thanks,
> Kyrill
>

I'm seeing:
gcc.target/arm/pr42574.c: syntax error in target selector
"arm_thumb1_ok && { ! arm_thumb1_movt_ok }" for " dg-do 1 compile {
arm_thumb1_ok && { ! arm_thumb1_movt_ok } } "

Can you fix it?

Thanks,

Christophe

>
>>
>> Best regards,
>>
>> Thomas
>>
>> On Friday 20 May 2016 12:14:44 Kyrill Tkachov wrote:
>>>
>>> Hi Thomas,
>>>
>>> On 19/05/16 17:11, Thomas Preudhomme wrote:

 On Wednesday 18 May 2016 12:30:41 Kyrill Tkachov wrote:
>
> Hi Thomas,
>
> This looks mostly good with a few nits inline.
> Please repost with the comments addressed.

 Updated ChangeLog entries:

 *** gcc/ChangeLog ***

 2016-05-18  Thomas Preud'homme  

   * config/arm/arm.h (TARGET_HAVE_MOVT): Include ARMv8-M as
 having
   MOVT.
   * config/arm/arm.c (arm_arch_name): (const_ok_for_op): Check
   MOVT/MOVW
   availability with TARGET_HAVE_MOVT.
   (thumb_legitimate_constant_p): Strip the high part of a
   label_ref.
   (thumb1_rtx_costs): Also return 0 if setting a half word
 constant
   and
   MOVW is available and replace (unsigned HOST_WIDE_INT) INTVAL
 by
   UINTVAL.
   (thumb1_size_rtx_costs): Make set of half word constant also
 cost
   1
   extra instruction if MOVW is available.  Use a cost variable
   incremented by COSTS_N_INSNS (1) when the condition match
 rather
   than
   returning an arithmetic expression based on 

Re: [PATCH, ARM 5/7] Add support for MOVT/MOVW to ARMv8-M Baseline

[Kyrill Tkachov]

Hi Thomas,

On 07/07/16 15:32, Thomas Preudhomme wrote:

Hi Kyrill,

Please find an updated version in attachment. Please note I made quite a few
other changes as well. The most important one was to add new ARMv8-M Baseline
only alternatives to the two movt insns in order to have non predicable output
template for ARMv8-M Baseline. I also inverted the logic for how to detect
ARMv8-M Baseline in the testsuite (arm_thumb1_movt_ok rather than *_ko) and
fixed a couple of typos. I didn't add MOVT test because it is difficult to
generate. Using -mslow-flash-data should help but it is not available for
ARMv8-M Baseline as only config/arm/thumb2.md was modified to support this
option.

Update ChangeLog entries are as follow:


*** gcc/ChangeLog ***

2016-06-20  Thomas Preud'homme  

 * config/arm/arm.h (TARGET_HAVE_MOVT): Include ARMv8-M as having MOVT.
 * config/arm/arm.c (arm_arch_name): (const_ok_for_op): Check MOVT/MOVW
 availability with TARGET_HAVE_MOVT.
 (thumb_legitimate_constant_p): Strip the high part of a label_ref.
 (thumb1_rtx_costs): Also return 0 if setting a half word constant and
 MOVW is available and replace (unsigned HOST_WIDE_INT) INTVAL by
 UINTVAL.
 (thumb1_size_rtx_costs): Make set of half word constant also cost 1
 extra instruction if MOVW is available.  Use a cost variable
 incremented by COSTS_N_INSNS (1) when the condition match rather than
 returning an arithmetic expression based on COSTS_N_INSNS.  Make
 constant with bottom half word zero cost 2 instruction if MOVW is
 available.
 * config/arm/arm.md (define_attr "arch"): Add v8mb.
 (define_attr "arch_enabled"): Set to yes if arch value is v8mb and
 target is ARMv8-M Baseline.
 (arm_movt): New unpredicable alternative for ARMv8-M Baseline.
 (arm_movtas_ze): Likewise.
 * config/arm/thumb1.md (thumb1_movdi_insn): Add ARMv8-M Baseline only
 alternative for constants satisfying j constraint.
 (thumb1_movsi_insn): Likewise.
 (movsi splitter for K alternative): Tighten condition to not trigger
 if movt is available and j constraint is satisfied.
 (Pe immediate splitter): Likewise.
 (thumb1_movhi_insn): Add ARMv8-M Baseline only alternative for
 constant fitting in an halfword to use MOVW.
 * doc/sourcebuild.texi (arm_thumb1_movt_ok): Document new ARM
 effective target.


*** gcc/testsuite/ChangeLog ***

2016-05-23  Thomas Preud'homme  

 * lib/target-supports.exp (check_effective_target_arm_thumb1_movt_ok):
 Define effective target.
 * gcc.target/arm/pr42574.c: Require arm_thumb1_ok and
 !arm_thumb1_movt_ok to exclude ARMv8-M Baseline.
 * gcc.target/arm/movhi_movw.c: New test.
 * gcc.target/arm/movsi_movw.c: Likewise.
 * gcc.target/arm/movdi_movw.c: Likewise.


Ok.
Thanks,
Kyrill



Best regards,

Thomas

On Friday 20 May 2016 12:14:44 Kyrill Tkachov wrote:

Hi Thomas,

On 19/05/16 17:11, Thomas Preudhomme wrote:

On Wednesday 18 May 2016 12:30:41 Kyrill Tkachov wrote:

Hi Thomas,

This looks mostly good with a few nits inline.
Please repost with the comments addressed.

Updated ChangeLog entries:

*** gcc/ChangeLog ***

2016-05-18  Thomas Preud'homme  

  * config/arm/arm.h (TARGET_HAVE_MOVT): Include ARMv8-M as having
  MOVT.
  * config/arm/arm.c (arm_arch_name): (const_ok_for_op): Check
  MOVT/MOVW
  availability with TARGET_HAVE_MOVT.
  (thumb_legitimate_constant_p): Strip the high part of a
  label_ref.
  (thumb1_rtx_costs): Also return 0 if setting a half word constant
  and
  MOVW is available and replace (unsigned HOST_WIDE_INT) INTVAL by
  UINTVAL.
  (thumb1_size_rtx_costs): Make set of half word constant also cost
  1
  extra instruction if MOVW is available.  Use a cost variable
  incremented by COSTS_N_INSNS (1) when the condition match rather
  than
  returning an arithmetic expression based on COSTS_N_INSNS.  Make
  constant with bottom half word zero cost 2 instruction if MOVW is
  available.
  * config/arm/arm.md (define_attr "arch"): Add v8mb.
  (define_attr "arch_enabled"): Set to yes if arch value is v8mb
  and
  target is ARMv8-M Baseline.
  * config/arm/thumb1.md (thumb1_movdi_insn): Add ARMv8-M Baseline
  only
  alternative for constants satisfying j constraint.
  (thumb1_movsi_insn): Likewise.
  (movsi splitter for K alternative): Tighten condition to not
  trigger
  if movt is available and j constraint is satisfied.
  (Pe immediate splitter): Likewise.
  (thumb1_movhi_insn): Add ARMv8-M Baseline only 

Re: [PATCH, ARM 5/7] Add support for MOVT/MOVW to ARMv8-M Baseline

[Thomas Preudhomme]

Hi Kyrill,

Please find an updated version in attachment. Please note I made quite a few 
other changes as well. The most important one was to add new ARMv8-M Baseline 
only alternatives to the two movt insns in order to have non predicable output 
template for ARMv8-M Baseline. I also inverted the logic for how to detect 
ARMv8-M Baseline in the testsuite (arm_thumb1_movt_ok rather than *_ko) and 
fixed a couple of typos. I didn't add MOVT test because it is difficult to 
generate. Using -mslow-flash-data should help but it is not available for 
ARMv8-M Baseline as only config/arm/thumb2.md was modified to support this 
option.

Update ChangeLog entries are as follow:


*** gcc/ChangeLog ***

2016-06-20  Thomas Preud'homme  

* config/arm/arm.h (TARGET_HAVE_MOVT): Include ARMv8-M as having MOVT.
* config/arm/arm.c (arm_arch_name): (const_ok_for_op): Check MOVT/MOVW
availability with TARGET_HAVE_MOVT.
(thumb_legitimate_constant_p): Strip the high part of a label_ref.
(thumb1_rtx_costs): Also return 0 if setting a half word constant and
MOVW is available and replace (unsigned HOST_WIDE_INT) INTVAL by
UINTVAL.
(thumb1_size_rtx_costs): Make set of half word constant also cost 1
extra instruction if MOVW is available.  Use a cost variable
incremented by COSTS_N_INSNS (1) when the condition match rather than
returning an arithmetic expression based on COSTS_N_INSNS.  Make
constant with bottom half word zero cost 2 instruction if MOVW is
available.
* config/arm/arm.md (define_attr "arch"): Add v8mb.
(define_attr "arch_enabled"): Set to yes if arch value is v8mb and
target is ARMv8-M Baseline.
(arm_movt): New unpredicable alternative for ARMv8-M Baseline.
(arm_movtas_ze): Likewise.
* config/arm/thumb1.md (thumb1_movdi_insn): Add ARMv8-M Baseline only
alternative for constants satisfying j constraint.
(thumb1_movsi_insn): Likewise.
(movsi splitter for K alternative): Tighten condition to not trigger
if movt is available and j constraint is satisfied.
(Pe immediate splitter): Likewise.
(thumb1_movhi_insn): Add ARMv8-M Baseline only alternative for
constant fitting in an halfword to use MOVW.
* doc/sourcebuild.texi (arm_thumb1_movt_ok): Document new ARM
effective target.


*** gcc/testsuite/ChangeLog ***

2016-05-23  Thomas Preud'homme  

* lib/target-supports.exp (check_effective_target_arm_thumb1_movt_ok):
Define effective target.
* gcc.target/arm/pr42574.c: Require arm_thumb1_ok and
!arm_thumb1_movt_ok to exclude ARMv8-M Baseline.
* gcc.target/arm/movhi_movw.c: New test.
* gcc.target/arm/movsi_movw.c: Likewise.
* gcc.target/arm/movdi_movw.c: Likewise.


Best regards,

Thomas

On Friday 20 May 2016 12:14:44 Kyrill Tkachov wrote:
> Hi Thomas,
> 
> On 19/05/16 17:11, Thomas Preudhomme wrote:
> > On Wednesday 18 May 2016 12:30:41 Kyrill Tkachov wrote:
> >> Hi Thomas,
> >> 
> >> This looks mostly good with a few nits inline.
> >> Please repost with the comments addressed.
> > 
> > Updated ChangeLog entries:
> > 
> > *** gcc/ChangeLog ***
> > 
> > 2016-05-18  Thomas Preud'homme  
> > 
> >  * config/arm/arm.h (TARGET_HAVE_MOVT): Include ARMv8-M as having
> >  MOVT.
> >  * config/arm/arm.c (arm_arch_name): (const_ok_for_op): Check
> >  MOVT/MOVW
> >  availability with TARGET_HAVE_MOVT.
> >  (thumb_legitimate_constant_p): Strip the high part of a
> >  label_ref.
> >  (thumb1_rtx_costs): Also return 0 if setting a half word constant
> >  and
> >  MOVW is available and replace (unsigned HOST_WIDE_INT) INTVAL by
> >  UINTVAL.
> >  (thumb1_size_rtx_costs): Make set of half word constant also cost
> >  1
> >  extra instruction if MOVW is available.  Use a cost variable
> >  incremented by COSTS_N_INSNS (1) when the condition match rather
> >  than
> >  returning an arithmetic expression based on COSTS_N_INSNS.  Make
> >  constant with bottom half word zero cost 2 instruction if MOVW is
> >  available.
> >  * config/arm/arm.md (define_attr "arch"): Add v8mb.
> >  (define_attr "arch_enabled"): Set to yes if arch value is v8mb
> >  and
> >  target is ARMv8-M Baseline.
> >  * config/arm/thumb1.md (thumb1_movdi_insn): Add ARMv8-M Baseline
> >  only
> >  alternative for constants satisfying j constraint.
> >  (thumb1_movsi_insn): Likewise.
> >  (movsi splitter for K alternative): Tighten condition to not
> >  trigger
> >  if movt is available and j constraint is satisfied.
> >  (Pe immediate splitter): Likewise.
> >