Re: [PATCH, ARM 5/7] Add support for MOVT/MOVW to ARMv8-M Baseline
On 10/10/16 13:35, Christophe Lyon wrote: Hi Thomas, Hi Christophe, On 13 July 2016 at 17:34, Thomas Preudhommewrote: 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
Hi Thomas, On 13 July 2016 at 17:34, Thomas Preudhommewrote: > 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
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
Hi Thomas, On 13 July 2016 at 12:01, Kyrill Tkachovwrote: > 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
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
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. > >