Message ID | 57EA7A64.5070709@foss.arm.com |
---|---|
State | New |
Headers | show |
Hi, On 27 September 2016 at 15:55, Kyrill Tkachov <kyrylo.tkachov@foss.arm.com> wrote: > Hi Matthew, > > > On 27/09/16 14:21, Matthew Wahab wrote: >> >> Recently added support for ARMv8.2-A >> (https://gcc.gnu.org/ml/gcc-patches/2016-05/msg01240.html) included a >> number of changes to improve data movement, particularly for HI and HF >> mode values. These included the use of the Thumb-2 instruction MOVW and >> of the new VMOV.F16 instruction. There are problems with both: the use >> of MOVW isn't properly guarded so that it can be generated for targets >> that don't support it and the VMOV.F16 instruction is wrongly marked as >> being predicable. >> >> This patch adds guards to the use of the MOVW so that it is only >> generated when the target supports Thumb-2 and fixes the predication on >> the VMOV.F16 instruction. >> >> Tested for arm-none-linux-gnueabihf with native bootstrap and make check >> on ARMv8-A and for arm-none-eabi with cross compiled check-gcc on an >> ARMv8.2-A emulator. >> >> There is one failure on arm-none-linux-gnueabihf, >> gcc.dg/guality/pr36728-1.c which is due to MOVW not being generated, >> even for ARMv7-A. The generated code is otherwise correct. I think I >> understand why MOVW isn't being emitted but it'll take time to test >> properly. >> >> Since this patch is to fix a broken build is it OK to commit it and to fix >> the poor code-gen in a follow-up patch? >> Matthew >> >> gcc/ >> 2016-09-27 Matthew Wahab <matthew.wahab@arm.com> >> >> * config/arm/arm.md (*arm_movsi_insn): Add "arch" attribute. >> * config/arm/vfp.md (*arm_movhi_vfp): Likewise. >> (*thumb2_movhi_vfp): Likewise. >> (*arm_movhi_fp16): Remove predication operand from VMOV.F16 >> template. Expand predicable attribute to mark VMOV.F16 as not >> predicable. Add "arch" attribute. >> (*thumb2_movhi_fp16): Likewise. >> (*arm_movsi_vfp): Break a long line. Add "arch" attribute. >> (*thumb2_movsi_vfp): Add "arch" attribute. > > > diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md > index 411754f..999292b 100644 > --- a/gcc/config/arm/arm.md > +++ b/gcc/config/arm/arm.md > @@ -6065,6 +6065,7 @@ > [(set_attr "type" "mov_reg,mov_imm,mvn_imm,mov_imm,load1,store1") > (set_attr "predicable" "yes") > (set_attr "pool_range" "*,*,*,*,4096,*") > + (set_attr "arch" "*,*,*,t2,*,*") > (set_attr "neg_pool_range" "*,*,*,*,4084,*")] > ) > > The "t2" attribute means that we're currently compiling for Thumb2. What you > want is to check that the architecture > we're compiling for supports Thumb2, so in this case you want the v6t2 > value. > In the interest of fixing the build I'll approve this patch as is since it's > still correct (just not as general as it could be), > but it'd be good to have a follow-up patch to fix this. >gcc.target/arm/constant-pool.c execution test > Thanks for fixing this, > Kyrill > > This patch fixes the regression I reported on gcc.target/arm/constant-pool.c gfortran.fortran-torture/execute/nan_inf_fmt.f90 as well as the compiler build failure I reported against patch 6/17 of your series. Thanks!
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md index 411754f..999292b 100644 --- a/gcc/config/arm/arm.md +++ b/gcc/config/arm/arm.md @@ -6065,6 +6065,7 @@ [(set_attr "type" "mov_reg,mov_imm,mvn_imm,mov_imm,load1,store1") (set_attr "predicable" "yes") (set_attr "pool_range" "*,*,*,*,4096,*") + (set_attr "arch" "*,*,*,t2,*,*") (set_attr "neg_pool_range" "*,*,*,*,4084,*")] )