| Message ID | AM4PR0701MB21629FE2DF6F5FA554B5E804E4B00@AM4PR0701MB2162.eurprd07.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Nov 18, 2016 at 3:50 PM, Bernd Edlinger <bernd.edlinger@hotmail.de> wrote: > On 11/18/16 12:58, Christophe Lyon wrote: >> On 17 November 2016 at 10:23, Kyrill Tkachov >> <kyrylo.tkachov@foss.arm.com> wrote: >>> >>> On 09/11/16 12:58, Bernd Edlinger wrote: >>>> >>>> Hi! >>>> >>>> >>>> This patch enables the ldrd/strd peephole rules unconditionally. >>>> >>>> It is meant to fix cases, where the patch to reduce the sha512 >>>> stack usage splits ldrd/strd instructions into separate ldr/str insns, >>>> but is technically independent from the other patch: >>>> >>>> See https://gcc.gnu.org/ml/gcc-patches/2016-11/msg00523.html >>>> >>>> It was necessary to change check_effective_target_arm_prefer_ldrd_strd >>>> to retain the true prefer_ldrd_strd tuning flag. >>>> >>>> >>>> Bootstrapped and reg-tested on arm-linux-gnueabihf. >>>> Is it OK for trunk? Hi Bernd, Any update on the other patch you mentioned? This one breaks bootstrap of arm-linux-gnueabihf with certain options like "--with-arch=armv7-a --with-fpu=neon --with-float=hard". I created PR78453 for tracking. Thanks, bin >>> >>> >>> This is ok. >>> Thanks, >>> Kyrill >>> >> >> Hi Bernd, >> >> Since you committed this patch (r242549), I'm seeing the new test >> failing on some arm*-linux-gnueabihf configurations: >> >> FAIL: gcc.target/arm/pr53447-5.c scan-assembler-times ldrd 10 >> FAIL: gcc.target/arm/pr53447-5.c scan-assembler-times strd 9 >> >> See http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/242549/report-build-info.html >> for a map of failures. >> >> Am I missing something? > > Hi Christophe, > > as always many thanks for your testing... > > I have apparently only looked at the case -mfloat-abi=soft here, which > is what my other patch is going to address. But all targets with > -mfpu=neon -mfloat-abi=hard can also use vldr.64 instead of ldrd > and vstr.64 instead of strd, which should be accepted as well. > > So the attached patch should fix at least most of the fallout. > > Is it OK for trunk? > > > Thanks > Bernd.
On 11/21/16 18:50, Bin.Cheng wrote: > Hi Bernd, > Any update on the other patch you mentioned? This one breaks > bootstrap of arm-linux-gnueabihf with certain options like > "--with-arch=armv7-a --with-fpu=neon --with-float=hard". > I created PR78453 for tracking. > > Thanks, > bin Oh, sorry. This should not depend on the other patch at all. Unfortunately I used --with-fpu=vfpv3-d16 where the bootstrap seems to work. I will try your configuration immediately. Bernd.
On 18 November 2016 at 16:50, Bernd Edlinger <bernd.edlinger@hotmail.de> wrote: > On 11/18/16 12:58, Christophe Lyon wrote: >> On 17 November 2016 at 10:23, Kyrill Tkachov >> <kyrylo.tkachov@foss.arm.com> wrote: >>> >>> On 09/11/16 12:58, Bernd Edlinger wrote: >>>> >>>> Hi! >>>> >>>> >>>> This patch enables the ldrd/strd peephole rules unconditionally. >>>> >>>> It is meant to fix cases, where the patch to reduce the sha512 >>>> stack usage splits ldrd/strd instructions into separate ldr/str insns, >>>> but is technically independent from the other patch: >>>> >>>> See https://gcc.gnu.org/ml/gcc-patches/2016-11/msg00523.html >>>> >>>> It was necessary to change check_effective_target_arm_prefer_ldrd_strd >>>> to retain the true prefer_ldrd_strd tuning flag. >>>> >>>> >>>> Bootstrapped and reg-tested on arm-linux-gnueabihf. >>>> Is it OK for trunk? >>> >>> >>> This is ok. >>> Thanks, >>> Kyrill >>> >> >> Hi Bernd, >> >> Since you committed this patch (r242549), I'm seeing the new test >> failing on some arm*-linux-gnueabihf configurations: >> >> FAIL: gcc.target/arm/pr53447-5.c scan-assembler-times ldrd 10 >> FAIL: gcc.target/arm/pr53447-5.c scan-assembler-times strd 9 >> >> See http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/242549/report-build-info.html >> for a map of failures. >> >> Am I missing something? > > Hi Christophe, > > as always many thanks for your testing... > > I have apparently only looked at the case -mfloat-abi=soft here, which > is what my other patch is going to address. But all targets with > -mfpu=neon -mfloat-abi=hard can also use vldr.64 instead of ldrd > and vstr.64 instead of strd, which should be accepted as well. > > So the attached patch should fix at least most of the fallout. > I've tested it, and indeed it fixes the failures I've reported. Thanks > Is it OK for trunk? > > > Thanks > Bernd.
Hi, does this follow-up patch look reasonable? See: https://gcc.gnu.org/ml/gcc-patches/2016-11/msg01945.html Is it OK for trunk? Thanks Bernd. On 11/21/16 21:46, Christophe Lyon wrote: > On 18 November 2016 at 16:50, Bernd Edlinger <bernd.edlinger@hotmail.de> wrote: >> On 11/18/16 12:58, Christophe Lyon wrote: >>> On 17 November 2016 at 10:23, Kyrill Tkachov >>> <kyrylo.tkachov@foss.arm.com> wrote: >>>> >>>> On 09/11/16 12:58, Bernd Edlinger wrote: >>>>> >>>>> Hi! >>>>> >>>>> >>>>> This patch enables the ldrd/strd peephole rules unconditionally. >>>>> >>>>> It is meant to fix cases, where the patch to reduce the sha512 >>>>> stack usage splits ldrd/strd instructions into separate ldr/str insns, >>>>> but is technically independent from the other patch: >>>>> >>>>> See https://gcc.gnu.org/ml/gcc-patches/2016-11/msg00523.html >>>>> >>>>> It was necessary to change check_effective_target_arm_prefer_ldrd_strd >>>>> to retain the true prefer_ldrd_strd tuning flag. >>>>> >>>>> >>>>> Bootstrapped and reg-tested on arm-linux-gnueabihf. >>>>> Is it OK for trunk? >>>> >>>> >>>> This is ok. >>>> Thanks, >>>> Kyrill >>>> >>> >>> Hi Bernd, >>> >>> Since you committed this patch (r242549), I'm seeing the new test >>> failing on some arm*-linux-gnueabihf configurations: >>> >>> FAIL: gcc.target/arm/pr53447-5.c scan-assembler-times ldrd 10 >>> FAIL: gcc.target/arm/pr53447-5.c scan-assembler-times strd 9 >>> >>> See http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/242549/report-build-info.html >>> for a map of failures. >>> >>> Am I missing something? >> >> Hi Christophe, >> >> as always many thanks for your testing... >> >> I have apparently only looked at the case -mfloat-abi=soft here, which >> is what my other patch is going to address. But all targets with >> -mfpu=neon -mfloat-abi=hard can also use vldr.64 instead of ldrd >> and vstr.64 instead of strd, which should be accepted as well. >> >> So the attached patch should fix at least most of the fallout. >> > > I've tested it, and indeed it fixes the failures I've reported. > > Thanks > >> Is it OK for trunk? >> >> >> Thanks >> Bernd. >> 2016-11-18 Bernd Edlinger <bernd.edlinger@hotmail.de> >> >> * gcc.target/arm/pr53447-5.c: Fix test expectations for neon-fpu. >> >>Index: gcc/testsuite/gcc.target/arm/pr53447-5.c >>=================================================================== >>--- gcc/testsuite/gcc.target/arm/pr53447-5.c (revision 242588) >>+++ gcc/testsuite/gcc.target/arm/pr53447-5.c (working copy) >>@@ -15,5 +15,8 @@ void foo(long long* p) >> p[9] -= p[10]; >> } >> >>-/* { dg-final { scan-assembler-times "ldrd" 10 } } */ >>-/* { dg-final { scan-assembler-times "strd" 9 } } */ >>+/* We accept neon instructions vldr.64 and vstr.64 as well. >>+ Note: DejaGnu counts patterns with alternatives twice, >>+ so actually there are only 10 loads and 9 stores. */ >>+/* { dg-final { scan-assembler-times "(ldrd|vldr\\.64)" 20 } } */ >>+/* { dg-final { scan-assembler-times "(strd|vstr\\.64)" 18 } } */
On 22/11/16 14:42, Bernd Edlinger wrote: > Hi, > > does this follow-up patch look reasonable? > See: https://gcc.gnu.org/ml/gcc-patches/2016-11/msg01945.html > > > Is it OK for trunk? > Ah yes, this one slipped my attention. This is ok. Thanks, Kyrill > Thanks > Bernd. > > On 11/21/16 21:46, Christophe Lyon wrote: >> On 18 November 2016 at 16:50, Bernd Edlinger <bernd.edlinger@hotmail.de> wrote: >>> On 11/18/16 12:58, Christophe Lyon wrote: >>>> On 17 November 2016 at 10:23, Kyrill Tkachov >>>> <kyrylo.tkachov@foss.arm.com> wrote: >>>>> On 09/11/16 12:58, Bernd Edlinger wrote: >>>>>> Hi! >>>>>> >>>>>> >>>>>> This patch enables the ldrd/strd peephole rules unconditionally. >>>>>> >>>>>> It is meant to fix cases, where the patch to reduce the sha512 >>>>>> stack usage splits ldrd/strd instructions into separate ldr/str insns, >>>>>> but is technically independent from the other patch: >>>>>> >>>>>> See https://gcc.gnu.org/ml/gcc-patches/2016-11/msg00523.html >>>>>> >>>>>> It was necessary to change check_effective_target_arm_prefer_ldrd_strd >>>>>> to retain the true prefer_ldrd_strd tuning flag. >>>>>> >>>>>> >>>>>> Bootstrapped and reg-tested on arm-linux-gnueabihf. >>>>>> Is it OK for trunk? >>>>> >>>>> This is ok. >>>>> Thanks, >>>>> Kyrill >>>>> >>>> Hi Bernd, >>>> >>>> Since you committed this patch (r242549), I'm seeing the new test >>>> failing on some arm*-linux-gnueabihf configurations: >>>> >>>> FAIL: gcc.target/arm/pr53447-5.c scan-assembler-times ldrd 10 >>>> FAIL: gcc.target/arm/pr53447-5.c scan-assembler-times strd 9 >>>> >>>> See http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/242549/report-build-info.html >>>> for a map of failures. >>>> >>>> Am I missing something? >>> Hi Christophe, >>> >>> as always many thanks for your testing... >>> >>> I have apparently only looked at the case -mfloat-abi=soft here, which >>> is what my other patch is going to address. But all targets with >>> -mfpu=neon -mfloat-abi=hard can also use vldr.64 instead of ldrd >>> and vstr.64 instead of strd, which should be accepted as well. >>> >>> So the attached patch should fix at least most of the fallout. >>> >> I've tested it, and indeed it fixes the failures I've reported. >> >> Thanks >> >>> Is it OK for trunk? >>> >>> >>> Thanks >>> Bernd. > >> 2016-11-18 Bernd Edlinger <bernd.edlinger@hotmail.de> > >> > >> * gcc.target/arm/pr53447-5.c: Fix test expectations for neon-fpu. > >> > >>Index: gcc/testsuite/gcc.target/arm/pr53447-5.c > >>=================================================================== > >>--- gcc/testsuite/gcc.target/arm/pr53447-5.c (revision 242588) > >>+++ gcc/testsuite/gcc.target/arm/pr53447-5.c (working copy) > >>@@ -15,5 +15,8 @@ void foo(long long* p) > >> p[9] -= p[10]; > >> } > >> > >>-/* { dg-final { scan-assembler-times "ldrd" 10 } } */ > >>-/* { dg-final { scan-assembler-times "strd" 9 } } */ > >>+/* We accept neon instructions vldr.64 and vstr.64 as well. > >>+ Note: DejaGnu counts patterns with alternatives twice, > >>+ so actually there are only 10 loads and 9 stores. */ > >>+/* { dg-final { scan-assembler-times "(ldrd|vldr\\.64)" 20 } } */ > >>+/* { dg-final { scan-assembler-times "(strd|vstr\\.64)" 18 } } */
2016-11-18 Bernd Edlinger <bernd.edlinger@hotmail.de> * gcc.target/arm/pr53447-5.c: Fix test expectations for neon-fpu. Index: gcc/testsuite/gcc.target/arm/pr53447-5.c =================================================================== --- gcc/testsuite/gcc.target/arm/pr53447-5.c (revision 242588) +++ gcc/testsuite/gcc.target/arm/pr53447-5.c (working copy) @@ -15,5 +15,8 @@ void foo(long long* p) p[9] -= p[10]; } -/* { dg-final { scan-assembler-times "ldrd" 10 } } */ -/* { dg-final { scan-assembler-times "strd" 9 } } */ +/* We accept neon instructions vldr.64 and vstr.64 as well. + Note: DejaGnu counts patterns with alternatives twice, + so actually there are only 10 loads and 9 stores. */ +/* { dg-final { scan-assembler-times "(ldrd|vldr\\.64)" 20 } } */ +/* { dg-final { scan-assembler-times "(strd|vstr\\.64)" 18 } } */