| Message ID | 20131219153809.GA18927@sale |
|---|---|
| State | New |
| Headers | show |
On 19/12/13 15:38, Charles Baylis wrote: > On Tue, Nov 26, 2013 at 10:25:50AM +0000, Richard Earnshaw wrote: >> I've just spotted another problem (that was always there): >>> @@ -42,15 +42,15 @@ >>> >>> (define_insn "*thumb_ldm4_ia" >>> [(match_parallel 0 "load_multiple_operation" >>> - [(set (match_operand:SI 1 "arm_hard_register_operand" "") >>> + [(set (match_operand:SI 1 "arm_hard_general_register_operand" "") >>> (mem:SI (match_operand:SI 5 "s_register_operand" "l"))) >>> - (set (match_operand:SI 2 "arm_hard_register_operand" "") >>> + (set (match_operand:SI 2 "arm_hard_general_register_operand" "") >>> (mem:SI (plus:SI (match_dup 5) >>> (const_int 4)))) >>> - (set (match_operand:SI 3 "arm_hard_register_operand" "") >>> + (set (match_operand:SI 3 "arm_hard_general_register_operand" "") >>> (mem:SI (plus:SI (match_dup 5) >>> (const_int 8)))) >>> - (set (match_operand:SI 4 "arm_hard_register_operand" "") >>> + (set (match_operand:SI 4 "arm_hard_general_register_operand" "") >>> (mem:SI (plus:SI (match_dup 5) >>> (const_int 12))))])] >>> "TARGET_THUMB1 && XVECLEN (operands[0], 0) == 4" >> >> This, and other thumb1 patterns should be stricter than this, since the >> T1 LDM/STM patterns only support low regs. I think these need changing >> to low_register_operand. > > I've also made this change. > > I've split this into 3 patches which must be applied in sequence. All patches > have been build tested for arm-unknown-linux-gnueabihf, and the 3 rolled > together have been bootstrapped on a Chromebook for arm-unknown-linux-gnueabihf. > > OK for trunk? > > > Changelog for: 0001-PR-target-59142-vfp_hard_register_operand.patch > > 2013-12-19 Charles Baylis <charles.baylis@linaro.org> > > PR target/59142 > gcc/ > * arm/predicates.md (vfp_hard_register_operand): New predicate. > * gcc/config/arm/arm.md: (vfp_pop_multiple_with_writeback) Use > vfp_hard_register_operand. > > > > > > Changelog for: 0002-PR-target-59142-arm_hard_general_register_operand.patch > > 2013-12-19 Charles Baylis <charles.baylis@linaro.org> > > PR target/59142 > gcc/ > * arm/predicates.md (arm_hard_general_register_operand): New predicate. > (arm_hard_register_operand): Remove. > * config/arm/arm-ldmstm.ml: Use arm_hard_general_register_operand > for all patterns. > * config/arm/ldmstm.md: Regenerate. > > > > > Changelog for: 0003-PR-target-59142-low_register_operand.patch > > 2013-12-19 Charles Baylis <charles.baylis@linaro.org> > > PR target/59142 > gcc/ > * config/arm/arm-ldmstm.ml: Use low_register_operand for Thumb > patterns. > * config/arm/ldmstm.md: Regenerate. > > Please fix the comment here: ;; Any hard register. -(define_predicate "arm_hard_register_operand" +(define_predicate "arm_hard_general_register_operand" Needs to be 'Any general register.' OK with that change. R.
On 19 December 2013 16:13, Richard Earnshaw <rearnsha@arm.com> wrote: > > OK with that change. Thanks. The bugzilla entry is targeted at 4.8, but it is a latent problem which affects 4.7 too. Is it ok for 4.8, and should it be considered for 4.7?
On 19/12/13 17:40, Charles Baylis wrote: > On 19 December 2013 16:13, Richard Earnshaw <rearnsha@arm.com> wrote: >> >> OK with that change. > > Thanks. > > The bugzilla entry is targeted at 4.8, but it is a latent problem > which affects 4.7 too. > > Is it ok for 4.8, and should it be considered for 4.7? > Yes, provided it passes testing on those releases. R.
On 20 December 2013 13:26, Richard Earnshaw <rearnsha@arm.com> wrote: > On 19/12/13 17:40, Charles Baylis wrote: >> Is it ok for 4.8, and should it be considered for 4.7? >> > > Yes, provided it passes testing on those releases. Results of testing 4.8: All 3 patches: 0001-PR-target-59142-vfp_hard_register_operand.patch 0002-PR-target-59142-arm_hard_general_register_operand.patch 0003-PR-target-59142-low_register_operand.patch apply correctly, and I have verified that ldmstm.md is correctly patched and does not need to be regenerated and have tested that the compiler bootstraps and passes make check in a arm-linux-gnueabihf configuration on a chromebook. Results of testing 4.7: Only the following 2 patches should be applied as patch 0001 modifies a pattern which does not exist on the 4.7 branch. 0002-PR-target-59142-arm_hard_general_register_operand.patch 0003-PR-target-59142-low_register_operand.patch I have verified that ldmstm.md is correctly patched and does not need to be regenerated and have tested that the compiler bootstraps in a arm-linux-gnueabi configuration on a chromebook. I think this is OK to be committed to both branches?
On 16/01/14 18:40, Charles Baylis wrote: > On 20 December 2013 13:26, Richard Earnshaw <rearnsha@arm.com> wrote: >> On 19/12/13 17:40, Charles Baylis wrote: >>> Is it ok for 4.8, and should it be considered for 4.7? >>> >> >> Yes, provided it passes testing on those releases. > > Results of testing 4.8: > All 3 patches: > 0001-PR-target-59142-vfp_hard_register_operand.patch > 0002-PR-target-59142-arm_hard_general_register_operand.patch > 0003-PR-target-59142-low_register_operand.patch > apply correctly, and I have verified that ldmstm.md is correctly > patched and does not need to be regenerated and have tested that the > compiler bootstraps and passes make check in a arm-linux-gnueabihf > configuration on a chromebook. > > > Results of testing 4.7: > Only the following 2 patches should be applied as patch 0001 modifies > a pattern which does not exist on the 4.7 branch. > 0002-PR-target-59142-arm_hard_general_register_operand.patch > 0003-PR-target-59142-low_register_operand.patch > I have verified that ldmstm.md is correctly patched and does not need > to be regenerated and have tested that the compiler bootstraps in a > arm-linux-gnueabi configuration on a chromebook. > > I think this is OK to be committed to both branches? > OK. R.
Committed on Charlies' behalf as: r206706 for the 4.8 branch r206707 for the 4.7 branch Christophe. On 17 January 2014 10:28, Richard Earnshaw <rearnsha@arm.com> wrote: > On 16/01/14 18:40, Charles Baylis wrote: >> On 20 December 2013 13:26, Richard Earnshaw <rearnsha@arm.com> wrote: >>> On 19/12/13 17:40, Charles Baylis wrote: >>>> Is it ok for 4.8, and should it be considered for 4.7? >>>> >>> >>> Yes, provided it passes testing on those releases. >> >> Results of testing 4.8: >> All 3 patches: >> 0001-PR-target-59142-vfp_hard_register_operand.patch >> 0002-PR-target-59142-arm_hard_general_register_operand.patch >> 0003-PR-target-59142-low_register_operand.patch >> apply correctly, and I have verified that ldmstm.md is correctly >> patched and does not need to be regenerated and have tested that the >> compiler bootstraps and passes make check in a arm-linux-gnueabihf >> configuration on a chromebook. >> >> >> Results of testing 4.7: >> Only the following 2 patches should be applied as patch 0001 modifies >> a pattern which does not exist on the 4.7 branch. >> 0002-PR-target-59142-arm_hard_general_register_operand.patch >> 0003-PR-target-59142-low_register_operand.patch >> I have verified that ldmstm.md is correctly patched and does not need >> to be regenerated and have tested that the compiler bootstraps in a >> arm-linux-gnueabi configuration on a chromebook. >> >> I think this is OK to be committed to both branches? >> > > OK. > > R. >
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md index 46fc442..5f7126d 100644 --- a/gcc/config/arm/arm.md +++ b/gcc/config/arm/arm.md @@ -12253,7 +12253,7 @@ [(set (match_operand:SI 1 "s_register_operand" "+rk") (plus:SI (match_dup 1) (match_operand:SI 2 "const_int_operand" "I"))) - (set (match_operand:DF 3 "arm_hard_register_operand" "") + (set (match_operand:DF 3 "vfp_hard_register_operand" "") (mem:DF (match_dup 1)))])] "TARGET_32BIT && TARGET_HARD_FLOAT && TARGET_VFP" "* diff --git a/gcc/config/arm/predicates.md b/gcc/config/arm/predicates.md index 29e1e5c..24f0548 100644 --- a/gcc/config/arm/predicates.md +++ b/gcc/config/arm/predicates.md @@ -98,6 +98,12 @@ && REGNO_REG_CLASS (REGNO (op)) == VFP_REGS))); }) +(define_predicate "vfp_hard_register_operand" + (match_code "reg") +{ + return (IS_VFP_REGNUM (REGNO (op))); +}) + (define_predicate "zero_operand" (and (match_code "const_int,const_double,const_vector") (match_test "op == CONST0_RTX (mode)")))