Re: [PATCH] PR69195, Reload confused by invalid reg equivs
On 03/12/2016 12:58 AM, Jakub Jelinek wrote:
On Sat, Mar 12, 2016 at 09:43:50AM +1030, Alan Modra wrote:
The underlying problem happens somewhere in tree-ssa-dse.c. So we get
an indirect jump to a random location instead of a jump to 0.
Well, the testcase is there just to make sure we don't ICE on it.
And, changing just DSE can't be a complete solution, because one can use
uninitialized var from the beginning:
int
foo (void)
{
int x;
goto *&x;
}
Jakub
I believe Alan's point is DSE deleted the assignment to x which can't be
right as long as we've left in goto *&x.
The goto *&x should be a use of x and thus should have kept the
assignment live.
Jeff
Re: [PATCH] PR69195, Reload confused by invalid reg equivs
On Sat, Mar 12, 2016 at 01:26:39AM -0700, Jeff Law wrote:
> On 03/12/2016 12:58 AM, Jakub Jelinek wrote:
> >On Sat, Mar 12, 2016 at 09:43:50AM +1030, Alan Modra wrote:
> >>The underlying problem happens somewhere in tree-ssa-dse.c. So we get
> >>an indirect jump to a random location instead of a jump to 0.
> >
> >Well, the testcase is there just to make sure we don't ICE on it.
> >And, changing just DSE can't be a complete solution, because one can use
> >uninitialized var from the beginning:
> >int
> >foo (void)
> >{
> > int x;
> > goto *&x;
> >}
> >
> > Jakub
> >
> I believe Alan's point is DSE deleted the assignment to x which can't be
> right as long as we've left in goto *&x.
>
> The goto *&x should be a use of x and thus should have kept the assignment
> live.
Right, I wasn't trying to say that ira.c:indirect_jump_optimize is
OK. It needs the patch I posted or perhaps even better a test of
DF_REF_INSN_INFO rather than !DF_REF_IS_ARTIFICIAL (simply because the
flag test is reading another field, and we need to read
DF_REF_INSN_INFO anyway).
--
Alan Modra
Australia Development Lab, IBM
Re: [PATCH] PR69195, Reload confused by invalid reg equivs
On Sat, Mar 12, 2016 at 07:37:25PM +1030, Alan Modra wrote: > > I believe Alan's point is DSE deleted the assignment to x which can't be > > right as long as we've left in goto *&x. > > > > The goto *&x should be a use of x and thus should have kept the assignment > > live. > > Right, I wasn't trying to say that ira.c:indirect_jump_optimize is > OK. It needs the patch I posted or perhaps even better a test of > DF_REF_INSN_INFO rather than !DF_REF_IS_ARTIFICIAL (simply because the > flag test is reading another field, and we need to read > DF_REF_INSN_INFO anyway). Ok, that was my point. BTW, DSE isn't the only one that deletes x = 0; cddce deletes it too. -fno-tree-dse -fno-tree-dce preserves it till expansion. Jakub
Re: [PATCH] PR69195, Reload confused by invalid reg equivs
On March 12, 2016 10:29:40 AM GMT+01:00, Jakub Jelinek wrote: >On Sat, Mar 12, 2016 at 07:37:25PM +1030, Alan Modra wrote: >> > I believe Alan's point is DSE deleted the assignment to x which >can't be >> > right as long as we've left in goto *&x. >> > >> > The goto *&x should be a use of x and thus should have kept the >assignment >> > live. >> >> Right, I wasn't trying to say that ira.c:indirect_jump_optimize is >> OK. It needs the patch I posted or perhaps even better a test of >> DF_REF_INSN_INFO rather than !DF_REF_IS_ARTIFICIAL (simply because >the >> flag test is reading another field, and we need to read >> DF_REF_INSN_INFO anyway). > >Ok, that was my point. BTW, DSE isn't the only one that deletes x = 0; >cddce deletes it too. -fno-tree-dse -fno-tree-dce preserves it till >expansion. GIMPLE_GOTO doesn't have VOPs and I don't think that we'd want VUSEs on all gotos. But just having them on indirect gotos would be inconsistent. I believe the code is undefined anyway and out of scope of a reasonable QOI. Using alloca to create/jump to code on the stack should work (we might transform that into a decl though). Richard. > Jakub
Fix stack checking emulation in Ada
When stack checking cannot be done by means of probes, typically because of the lack of MMU, the Ada compiler emulates it in the runtime (that's required if you want to pass the ACATS testsuite) through calls to a specific routine. Now calls to this routine are emitted as libcalls and libcalls are ECF_NOTHROW by default: /* By default, library functions cannot throw. */ flags = ECF_NOTHROW; so this is problematic with optimization enabled because the compiler needs to catch the Storage_Error exception for ACATS. That's why the attached patch changes probe_stack_range to use a LCT_THROW libcall instead and also tweaks emit_library_call_value_1 not to set ECF_NORETURN for it (the stack checking routine obviously returns most of the time and throws only exceptionally). This only affects the Ada compiler since stack_check_libfunc is set only in Ada and LCT_THROW has been unused since 2009. That's an old issue which recently resurfaced under PR ada/70017 because s390/Linux was still using stack checking emulation; the PR was addressed for GCC 6 by switching to real stack checking but I have applied the patch on the mainline for the other platforms still using emulation, after testing it on x86-64/Linux. 2016-03-12 Eric Botcazou PR ada/70017 * calls.c (emit_library_call_value_1): Clear the ECF_NOTHROW flag if the libcall is LCT_THROW. * explow.c (probe_stack_range): Pass LCT_THROW to emit_library_call for the checking routine. -- Eric BotcazouIndex: calls.c === --- calls.c (revision 234098) +++ calls.c (working copy) @@ -3852,7 +3852,7 @@ emit_library_call_value_1 (int retval, r reg_parm_stack_space = REG_PARM_STACK_SPACE ((tree) 0); #endif - /* By default, library functions can not throw. */ + /* By default, library functions cannot throw. */ flags = ECF_NOTHROW; switch (fn_type) @@ -3869,7 +3869,7 @@ emit_library_call_value_1 (int retval, r flags |= ECF_NORETURN; break; case LCT_THROW: - flags = ECF_NORETURN; + flags &= ~ECF_NOTHROW; break; case LCT_RETURNS_TWICE: flags = ECF_RETURNS_TWICE; Index: explow.c === --- explow.c (revision 234098) +++ explow.c (working copy) @@ -1566,7 +1566,7 @@ probe_stack_range (HOST_WIDE_INT first, stack_pointer_rtx, plus_constant (Pmode, size, first))); - emit_library_call (stack_check_libfunc, LCT_NORMAL, VOIDmode, 1, addr, + emit_library_call (stack_check_libfunc, LCT_THROW, VOIDmode, 1, addr, Pmode); }
[PATCH] rs6000: Handle "d" output in the bd*z patterns (PR70098)
In the rs6000 port, FLOAT_REGS can contain DImode values when compiling for 64-bit targets. Some instructions (like "fcfid" in the testcase, convert from integer to DP float) only work on floating point registers. So, we do want to allow DImode in these regs. Now, in unusual cases IRA will assign FLOAT_REGS to some allocno where some insns cannot handle FLOAT_REGS there, so they will need a reload. Maybe IRA can be made smarter, but it isn't doing anything wrong here, so we should be able to handle it. The place it goes wrong is in the output of the *ctrdi_internal[1256] pattern: the "bdz" and "bdnz" instructions. GCC refuses to do output reloads on JUMP_INSNs, probably because it is hard to do, needs different strategies than "normal" reloads do, and it cannot even be done at all for general patterns. So JUMP_INSNs need to be able to handle every possible output for the register class used. These patterns already handle writing to "c" (the base insn case), and to "r", "m", and "c" or "l"; all those via splitters. We just need to handle "d" as well. That is what this patch does. [A predicate in one of the splitters needs to be touched up so that the correct splitter is used for the FLOAT_REGS case.] But, that leaves another problem. One of the insns that are split to is a move from a GPR to an FPR. That work fine on targets with direct move (which does exactly that), i.e. power8 and up. But older targets need memory to do the move, and this splitter runs after reload so it cannot allocate memory; and allocating memory beforehand for every bdnz insn is pretty horrible as well. This patch implements the easy part. With it, power8 works, where it didn't before. Tested on powerpc64-linux, -m32 and -m64, -mlra and -mno-lra. Also regstrapping on a power8 powerpc64le-linux. Is this okay for trunk if that works as expected? Segher 2016-03-12 Segher Boessenkool PR target/70098 * config/rs6000/rs6000.md (*ctr_internal1, *ctr_internal2, *ctr_internal5, *ctr_internal6): Also allow "d" as output. (define_split for the GPR case): Use int_reg_operand instead of gpc_reg_operand for the output. gcc/testsuite/ PR target/70098 * g++.dg/pr70098.C: New testcase. --- gcc/config/rs6000/rs6000.md| 10 ++--- gcc/testsuite/g++.dg/pr70098.C | 89 ++ 2 files changed, 94 insertions(+), 5 deletions(-) create mode 100644 gcc/testsuite/g++.dg/pr70098.C diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md index c92c868..f5337dd 100644 --- a/gcc/config/rs6000/rs6000.md +++ b/gcc/config/rs6000/rs6000.md @@ -11909,7 +11909,7 @@ (define_insn "*ctr_internal1" (const_int 1)) (label_ref (match_operand 0 "" "")) (pc))) - (set (match_operand:P 2 "nonimmediate_operand" "=1,*r,m,*c*l") + (set (match_operand:P 2 "nonimmediate_operand" "=1,*r,m,*d*c*l") (plus:P (match_dup 1) (const_int -1))) (clobber (match_scratch:CC 3 "=X,&x,&x,&x")) @@ -11933,7 +11933,7 @@ (define_insn "*ctr_internal2" (const_int 1)) (pc) (label_ref (match_operand 0 "" "" - (set (match_operand:P 2 "nonimmediate_operand" "=1,*r,m,*c*l") + (set (match_operand:P 2 "nonimmediate_operand" "=1,*r,m,*d*c*l") (plus:P (match_dup 1) (const_int -1))) (clobber (match_scratch:CC 3 "=X,&x,&x,&x")) @@ -11959,7 +11959,7 @@ (define_insn "*ctr_internal5" (const_int 1)) (label_ref (match_operand 0 "" "")) (pc))) - (set (match_operand:P 2 "nonimmediate_operand" "=1,*r,m,*c*l") + (set (match_operand:P 2 "nonimmediate_operand" "=1,*r,m,*d*c*l") (plus:P (match_dup 1) (const_int -1))) (clobber (match_scratch:CC 3 "=X,&x,&x,&x")) @@ -11983,7 +11983,7 @@ (define_insn "*ctr_internal6" (const_int 1)) (pc) (label_ref (match_operand 0 "" "" - (set (match_operand:P 2 "nonimmediate_operand" "=1,*r,m,*c*l") + (set (match_operand:P 2 "nonimmediate_operand" "=1,*r,m,*d*c*l") (plus:P (match_dup 1) (const_int -1))) (clobber (match_scratch:CC 3 "=X,&x,&x,&x")) @@ -12010,7 +12010,7 @@ (define_split (const_int 1)]) (match_operand 5 "" "") (match_operand 6 "" ""))) - (set (match_operand:P 0 "gpc_reg_operand" "") + (set (match_operand:P 0 "int_reg_operand" "") (plus:P (match_dup 1) (const_int -1))) (clobber (match_scratch:CC 3 "")) (clobber (match_scratch:P 4 ""))] diff --git a/gcc/testsuite/g++.dg/pr70098.C b/gcc/testsuite/g++.dg/pr70098.C new file mode 100644 index 000..c7b4f63 --- /dev/null +++ b/gcc/testsuite/g++.dg/pr70098.C @@ -0,0 +1,89 @@ +// PR target
Re: [Patch, fortran] PRs 70031 and 69524 - submodule tweaks
Hi Jerry, Thanks - committed as revision 234161. All the best Paul On 8 March 2016 at 00:47, Jerry DeLisle wrote: > On 03/06/2016 10:18 AM, Paul Richard Thomas wrote: >> Dear All, >> >> These are two rather trivial modifications to permit, 'module' to >> appear at any position in the list of prefixes in the procedure >> declaration and to allow module procedures to appear within a module >> contains section. I was rather astonished at this latter since it does >> seem to be rather contrary to having an module interface declaration >> for the same procedure. However, from the Fortran 2008 standard: >> >> C1247 (R1225) MODULE shall appear only in the function-stmt or >> subroutine-stmt of a module subprogram or of a nonabstract interface >> body that is declared in the scoping unit of a module or submodule. >> >> Whilst I was about it, I prevented an ICE from occurring following the >> error generated by decl.c(copy_prefix), when prefixes in the interface >> are repeated in the procedure declaration. I have not included a test >> for this, since I am not convinced that repeating the prefixes is >> strictly speaking an error. In fact, it would make a lot of sense to >> repeat the interface declaration completely in the submodule >> declaration. I will investigate further before committing. The fix is >> even more trivial than preventing the ICE. >> >> Since the patch is entirely permissive, it will not prevent correct >> code from compiling. In this sense, it is safe for stage 4. >> >> Bootstrapped and regtested on FC21/x86_64. OK for trunk? >> >> Best regards >> >> Paul >> > OK Paul, thanks for work. > > Jerry -- The difference between genius and stupidity is; genius has its limits. Albert Einstein
patch for PR69614
The following patch should solve the PR which is discussed on
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69614
The patch was bootstrapped and tested on x86/x86-64.
Committed as rev. 234162.
Index: ChangeLog
===
--- ChangeLog (revision 234142)
+++ ChangeLog (working copy)
@@ -1,3 +1,9 @@
+2016-03-12 Vladimir Makarov
+
+ PR target/69614
+ * lra-constraints.c (delete_move_and_clobber): New.
+ (remove_inheritance_pseudos): Use it.
+
2016-03-11 Kyrylo Tkachov
PR target/70002
Index: testsuite/ChangeLog
===
--- testsuite/ChangeLog (revision 234142)
+++ testsuite/ChangeLog (working copy)
@@ -1,3 +1,8 @@
+2016-03-12 Vladimir Makarov
+
+ PR target/69614
+ * gcc.target/arm/pr69614.c: New.
+
2016-03-11 Kyrylo Tkachov
* gcc.target/aarch64/vect-reduc-or_1.c: Add -fno-vect-cost-model to
Index: lra-constraints.c
===
--- lra-constraints.c (revision 234142)
+++ lra-constraints.c (working copy)
@@ -5850,6 +5850,24 @@ get_regno (rtx reg)
return -1;
}
+/* Delete a move INSN with destination reg DREGNO and a previous
+ clobber insn with the same regno. The inheritance/split code can
+ generate moves with preceding clobber and when we delete such moves
+ we should delete the clobber insn too to keep the correct life
+ info. */
+static void
+delete_move_and_clobber (rtx_insn *insn, int dregno)
+{
+ rtx_insn *prev_insn = PREV_INSN (insn);
+
+ lra_set_insn_deleted (insn);
+ lra_assert (dregno > 0);
+ if (prev_insn != NULL && NONDEBUG_INSN_P (prev_insn)
+ && GET_CODE (PATTERN (prev_insn)) == CLOBBER
+ && dregno == get_regno (XEXP (PATTERN (prev_insn), 0)))
+lra_set_insn_deleted (prev_insn);
+}
+
/* Remove inheritance/split pseudos which are in REMOVE_PSEUDOS and
return true if we did any change. The undo transformations for
inheritance looks like
@@ -5922,7 +5940,7 @@ remove_inheritance_pseudos (bitmap remov
? "split" : "inheritance");
dump_insn_slim (lra_dump_file, curr_insn);
}
- lra_set_insn_deleted (curr_insn);
+ delete_move_and_clobber (curr_insn, dregno);
done_p = true;
}
else if (bitmap_bit_p (remove_pseudos, sregno)
@@ -6122,7 +6140,7 @@ undo_optional_reloads (void)
INSN_UID (insn));
dump_insn_slim (lra_dump_file, insn);
}
- lra_set_insn_deleted (insn);
+ delete_move_and_clobber (insn, REGNO (dest));
continue;
}
/* We should not worry about generation memory-memory
Index: testsuite/gcc.target/arm/pr69614.c
===
--- testsuite/gcc.target/arm/pr69614.c (revision 0)
+++ testsuite/gcc.target/arm/pr69614.c (working copy)
@@ -0,0 +1,39 @@
+/* { dg-do run } */
+/* { dg-require-effective-target arm_arch_v7a_ok } */
+/* { dg-options "-Os -w -fno-expensive-optimizations -fschedule-insns -mtpcs-leaf-frame -fira-algorithm=priority" } */
+
+
+typedef unsigned short u16;
+typedef unsigned short v16u16 __attribute__ ((vector_size (16)));
+typedef unsigned int u32;
+typedef unsigned int v16u32 __attribute__ ((vector_size (16)));
+typedef unsigned long long u64;
+typedef unsigned long long v16u64 __attribute__ ((vector_size (16)));
+
+u64 __attribute__ ((noinline, noclone))
+foo(u16 u16_0, u32 u32_0, u64 u64_0, u16 u16_1, u32 u32_1, u64 u64_1,
+v16u16 v16u16_0, v16u32 v16u32_0, v16u64 v16u64_0, v16u16 v16u16_1, v16u32 v16u32_1, v16u64 v16u64_1)
+{
+ v16u64_0 %= (v16u64){(u16) v16u16_0[5], ~v16u64_1[1]};
+ v16u64_0[1] = 1;
+ v16u32_1[3] >>= 31;
+ v16u64_1 ^= (v16u64){v16u16_1[4], u64_1};
+ v16u64_1[0] = (v16u64_1[0] >> 63) | (v16u64_1[0] << 1);
+ u16_0 -= 1;
+ v16u32_1 %= (v16u32)-v16u64_0 | 1;
+ v16u16_0 /= (v16u16){-u64_1} | 1;
+ v16u32_0[2] |= (u16)~u16_1;
+return u16_0 + u64_0 + u32_1 + u64_1 +
+v16u16_0[0] + v16u16_0[1] + v16u16_0[2] + v16u16_0[3] + v16u16_0[4] + v16u16_0[5] + v16u16_0[6] + v16u32_0[2] + v16u32_0[3] + v16u64_0[0] +
+ v16u16_1[2] + v16u16_1[4] + v16u32_1[0] + v16u32_1[1] + v16u32_1[2] + v16u32_1[3] + v16u64_1[0] + v16u64_1[1];
+}
+
+int
+main ()
+{
+ u64 x = foo(0, 0, 1, 0, 0, 1, (v16u16){-1, 0, 0, 0, 0, 1}, (v16u32){0}, (v16u64){0}, (v16u16){0}, (v16u32){0}, (v16u64){0x67784fdb22, 1});
+ __builtin_printf ("%016llx\n", (unsigned long long) (x >> 0));
+ if (x != 0x00cef0a1b646)
+__builtin_abort();
+ return 0;
+}
Re: [PATCH] rs6000: Handle "d" output in the bd*z patterns (PR70098)
On Sat, Mar 12, 2016 at 8:55 AM, Segher Boessenkool wrote: > In the rs6000 port, FLOAT_REGS can contain DImode values when compiling > for 64-bit targets. Some instructions (like "fcfid" in the testcase, > convert from integer to DP float) only work on floating point registers. > So, we do want to allow DImode in these regs. > > Now, in unusual cases IRA will assign FLOAT_REGS to some allocno where > some insns cannot handle FLOAT_REGS there, so they will need a reload. > Maybe IRA can be made smarter, but it isn't doing anything wrong here, > so we should be able to handle it. > > The place it goes wrong is in the output of the *ctrdi_internal[1256] > pattern: the "bdz" and "bdnz" instructions. GCC refuses to do output > reloads on JUMP_INSNs, probably because it is hard to do, needs different > strategies than "normal" reloads do, and it cannot even be done at all > for general patterns. So JUMP_INSNs need to be able to handle every > possible output for the register class used. > > These patterns already handle writing to "c" (the base insn case), and > to "r", "m", and "c" or "l"; all those via splitters. We just need to > handle "d" as well. That is what this patch does. [A predicate in one > of the splitters needs to be touched up so that the correct splitter > is used for the FLOAT_REGS case.] > > But, that leaves another problem. One of the insns that are split to > is a move from a GPR to an FPR. That work fine on targets with direct > move (which does exactly that), i.e. power8 and up. But older targets > need memory to do the move, and this splitter runs after reload so > it cannot allocate memory; and allocating memory beforehand for every > bdnz insn is pretty horrible as well. > > This patch implements the easy part. With it, power8 works, where it > didn't before. > > Tested on powerpc64-linux, -m32 and -m64, -mlra and -mno-lra. Also > regstrapping on a power8 powerpc64le-linux. Is this okay for trunk > if that works as expected? > > > Segher > > > 2016-03-12 Segher Boessenkool > > PR target/70098 > * config/rs6000/rs6000.md (*ctr_internal1, *ctr_internal2, > *ctr_internal5, *ctr_internal6): Also allow "d" as output. > (define_split for the GPR case): Use int_reg_operand instead of > gpc_reg_operand for the output. > > gcc/testsuite/ > PR target/70098 > * g++.dg/pr70098.C: New testcase. This is okay. The testcase will need some XFAILs. Thanks, David
Re: patch for PR69614
On Sat, Mar 12, 2016 at 09:56:54AM -0500, Vladimir Makarov wrote:
> +2016-03-12 Vladimir Makarov
> +
> + PR target/69614
> + * lra-constraints.c (delete_move_and_clobber): New.
> + (remove_inheritance_pseudos): Use it.
Thanks for working on this.
> --- lra-constraints.c (revision 234142)
> +++ lra-constraints.c (working copy)
> @@ -5850,6 +5850,24 @@ get_regno (rtx reg)
>return -1;
> }
>
> +/* Delete a move INSN with destination reg DREGNO and a previous
> + clobber insn with the same regno. The inheritance/split code can
> + generate moves with preceding clobber and when we delete such moves
> + we should delete the clobber insn too to keep the correct life
> + info. */
> +static void
> +delete_move_and_clobber (rtx_insn *insn, int dregno)
> +{
> + rtx_insn *prev_insn = PREV_INSN (insn);
> +
> + lra_set_insn_deleted (insn);
> + lra_assert (dregno > 0);
> + if (prev_insn != NULL && NONDEBUG_INSN_P (prev_insn)
> + && GET_CODE (PATTERN (prev_insn)) == CLOBBER
> + && dregno == get_regno (XEXP (PATTERN (prev_insn), 0)))
> +lra_set_insn_deleted (prev_insn);
> +}
I just wonder if this isn't at least a latent -fcompare-debug issue.
Using prev_nondebug_insn (insn) instead of PREV_INSN (insn) would look
safer. If you are sure this is always an insn generated during LRA rather
than preexisting, and there is no chance DEBUG_INSNs could appear in
between, please ignore me.
Jakub
Re: [PATCH] PR69195, Reload confused by invalid reg equivs
On 03/12/2016 04:10 AM, Richard Biener wrote: On March 12, 2016 10:29:40 AM GMT+01:00, Jakub Jelinek wrote: On Sat, Mar 12, 2016 at 07:37:25PM +1030, Alan Modra wrote: I believe Alan's point is DSE deleted the assignment to x which can't be right as long as we've left in goto *&x. The goto *&x should be a use of x and thus should have kept the assignment live. Right, I wasn't trying to say that ira.c:indirect_jump_optimize is OK. It needs the patch I posted or perhaps even better a test of DF_REF_INSN_INFO rather than !DF_REF_IS_ARTIFICIAL (simply because the flag test is reading another field, and we need to read DF_REF_INSN_INFO anyway). Ok, that was my point. BTW, DSE isn't the only one that deletes x = 0; cddce deletes it too. -fno-tree-dse -fno-tree-dce preserves it till expansion. GIMPLE_GOTO doesn't have VOPs and I don't think that we'd want VUSEs on all gotos. But just having them on indirect gotos would be inconsistent. I believe the code is undefined anyway and out of scope of a reasonable QOI. Undefined? Most likely. But we still have to do something sensible. As Jakub noted, a user could create the problematic code just as easily as DCE/DSE, so IRA probably needs to be tolerant of this situation. So it seems like you're suggesting we leave DCE/DSE alone (declaring this usage undefined) and fix IRA to be tolerant, right? Using alloca to create/jump to code on the stack should work (we might transform that into a decl though). Given that executable stacks are a huge security hole, I'd be willing to go out on a limb and declare that undefined as well. It's not as clear cut, but that's the argument I'd make. And yes, I realize that goes in opposition to what GCC has allowed for 20+ years. I still think it'd be the right thing to do. jeff
[PATCH][PR rtl-optimization/69307] Handle hard registers in modes that span more than one register properly
As Andrey outlined in the PR, selective-scheduling was missing a check &
handling of hard registers in modes that span more than one hard reg.
This caused an incorrect register selection during renaming.
I verified removing the printf call from the test would not compromise
the test. Then I did a normal x86 bootstrap & regression test with the
patch. Of course that's essentially useless, so I also did another
bootstrap and regression test with -fselective-scheduling in BOOT_CFLAGS
with and without this patch. In both cases there were no regressions.
I'm installing Andrey's patch on the trunk. I'm not sure this is worth
addressing in gcc-5.
Jeff
commit 7965fd4e4fdd06addbca2f4a92657df6cadbd002
Author: law
Date: Sat Mar 12 17:12:29 2016 +
PR rtl-optimization/69307
* sel-sched.c (choose_best_pseudo_reg): Properly check for hard
registers in modes that span more than one register.
PR rtl-optimization/69307
* gcc.dg/pr69307.c: New test.
git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@234163
138bc75d-0d04-0410-961f-82ee72b054a4
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 7d73d32..6c41cf0 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@
+2016-03-12 Andrey Belevantsev
+
+ PR rtl-optimization/69307
+ * sel-sched.c (choose_best_pseudo_reg): Properly check for hard
+ registers in modes that span more than one register.
+
2016-03-12 Vladimir Makarov
PR target/69614
diff --git a/gcc/sel-sched.c b/gcc/sel-sched.c
index bd32ab5..09cf028 100644
--- a/gcc/sel-sched.c
+++ b/gcc/sel-sched.c
@@ -1457,31 +1457,44 @@ choose_best_pseudo_reg (regset used_regs,
gcc_assert (mode == GET_MODE (dest));
orig_regno = REGNO (dest);
- if (!REGNO_REG_SET_P (used_regs, orig_regno))
-{
- if (orig_regno < FIRST_PSEUDO_REGISTER)
-{
- gcc_assert (df_regs_ever_live_p (orig_regno));
+ /* Check that nothing in used_regs intersects with orig_regno. When
+we have a hard reg here, still loop over hard_regno_nregs. */
+ if (HARD_REGISTER_NUM_P (orig_regno))
+ {
+ int j, n;
+ for (j = 0, n = hard_regno_nregs[orig_regno][mode]; j < n; j++)
+ if (REGNO_REG_SET_P (used_regs, orig_regno + j))
+ break;
+ if (j < n)
+ continue;
+ }
+ else
+ {
+ if (REGNO_REG_SET_P (used_regs, orig_regno))
+ continue;
+ }
+ if (HARD_REGISTER_NUM_P (orig_regno))
+ {
+ gcc_assert (df_regs_ever_live_p (orig_regno));
- /* For hard registers, we have to check hardware imposed
- limitations (frame/stack registers, calls crossed). */
- if (!TEST_HARD_REG_BIT (reg_rename_p->unavailable_hard_regs,
- orig_regno))
- {
- /* Don't let register cross a call if it doesn't already
-cross one. This condition is written in accordance with
-that in sched-deps.c sched_analyze_reg(). */
- if (!reg_rename_p->crosses_call
- || REG_N_CALLS_CROSSED (orig_regno) > 0)
- return gen_rtx_REG (mode, orig_regno);
- }
+ /* For hard registers, we have to check hardware imposed
+limitations (frame/stack registers, calls crossed). */
+ if (!TEST_HARD_REG_BIT (reg_rename_p->unavailable_hard_regs,
+ orig_regno))
+ {
+ /* Don't let register cross a call if it doesn't already
+cross one. This condition is written in accordance with
+that in sched-deps.c sched_analyze_reg(). */
+ if (!reg_rename_p->crosses_call
+ || REG_N_CALLS_CROSSED (orig_regno) > 0)
+ return gen_rtx_REG (mode, orig_regno);
+ }
- bad_hard_regs = true;
-}
- else
-return dest;
-}
- }
+ bad_hard_regs = true;
+ }
+ else
+ return dest;
+}
*is_orig_reg_p_ptr = false;
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 4ef53c4..1334712 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2016-03-12 Andrey Belevantsev
+
+ PR rtl-optimization/69307
+ * gcc.dg/pr69307.c: New test.
+
2016-03-12 Vladimir Makarov
PR target/69614
diff --git a/gcc/testsuite/gcc.dg/pr69307.c b/gcc/testsuite/gcc.dg/pr69307.c
new file mode 100644
index 000..d9d343e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr69307.c
@@ -0,0 +1,34 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -fselective-scheduling2" } */
+
+typedef unsigned char uint8_t;
+typedef unsigned short int uint16_t;
+typedef unsigned int uint32_t;
+typedef unsigned long long int uint64_t;
+typedef uint8_t u8;
+typedef uint16_t u1
Re: patch for PR69614
On 03/12/2016 07:56 AM, Vladimir Makarov wrote: The following patch should solve the PR which is discussed on https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69614 The patch was bootstrapped and tested on x86/x86-64. Committed as rev. 234162. Isn't this potentially a problem for the gcc-4.9 and gcc-5 release branches? I'm going to add regression markers for those and drop the gcc-6 regression marker. jeff
[patch,fortran] PR69043 Trying to include a directory causes an infinite loop
I plan to commit the attached patch and test case under simple and obvious
tomorrow.
Regression tested on x86-64-linux.
Regards,
Jerry
2016-03-13 Jerry DeLisle
Jim MacArthur
PR fortran/69043
* scanner.c (load_file): Check that included file is not a directory.
diff --git a/gcc/fortran/scanner.c b/gcc/fortran/scanner.c
index c4e7974..d4c14bc 100644
--- a/gcc/fortran/scanner.c
+++ b/gcc/fortran/scanner.c
@@ -2200,6 +2200,8 @@ load_file (const char *realfilename, const char *displayedname, bool initial)
FILE *input;
int len, line_len;
bool first_line;
+ struct stat st;
+ int stat_result;
const char *filename;
/* If realfilename and displayedname are different and non-null then
surely realfilename is the preprocessed form of
@@ -2227,6 +2229,7 @@ load_file (const char *realfilename, const char *displayedname, bool initial)
}
else
input = gfc_open_file (realfilename);
+
if (input == NULL)
{
gfc_error_now ("Can't open file %qs", filename);
@@ -2242,6 +2245,16 @@ load_file (const char *realfilename, const char *displayedname, bool initial)
current_file->filename, current_file->line, filename);
return false;
}
+
+ stat_result = stat (realfilename, &st);
+ if (stat_result == 0 && st.st_mode & S_IFDIR)
+ {
+ fprintf (stderr, "%s:%d: Error: Included path '%s'"
+ " is a directory.\n",
+ current_file->filename, current_file->line, filename);
+ fclose (input);
+ return false;
+ }
}
/* Load the file.
! { dg-do compile }
! PR69043 Trying to include a directory causes an infinite loop
include '.'
program main
end program
! { dg-error "is a directory" " " { target *-*-* } 3 }
[DOC Patch] Add sample for @cc constraint
The docs for the new(-ish) @cc constraint need an example. Attached.
ChangeLog:
2016-03-12 David Wohlferd
* doc/extend.texi: Add sample for @cc constraint
Note that while I have a release on file with FSF, I don't have write
access to SVN.
dw
Index: extend.texi
===
--- extend.texi (revision 234163)
+++ extend.texi (working copy)
@@ -8047,6 +8047,7 @@
Because of the special nature of the flag output operands, the constraint
may not include alternatives.
+Do not clobber flags if they are being used as outputs.
Most often, the target has only one flags register, and thus is an implied
operand of many instructions. In this case, the operand should not be
@@ -8105,6 +8106,43 @@
``not'' @var{flag}, or inverted versions of those above
@end table
+For builds that don't support flag output operands, this example generates
+code to convert the flags to a byte (@code{setc}), then converts the
+byte back to flags (@code{if (a)} would generate a @code{testb}):
+
+@example
+char a;
+
+#ifndef __GCC_ASM_FLAG_OUTPUTS__
+
+/* If outputting flags is not supported. */
+asm ("bt $0, %1\n\t"
+ "setc %0"
+ : "=q" (a)
+ : "r" (b)
+ : "cc");
+
+#else
+
+/* Avoid the redundant setc/testb and use the carry flag directly. */
+asm ("bt $0, %1"
+ : "=@@ccc" (a)
+ : "r" (b));
+
+#endif
+
+if (a)
+ printf ("odd\n");
+else
+ printf ("even\n");
+@end example
+
+To see the improvement in the generated output, make sure optimizations
+are enabled.
+
+Note: On the x86 platform, flags are normally considered clobbered by
+extended asm whether the @code{"cc"} clobber is specified or not.
+
@end table
@anchor{InputOperands}
@@ -8260,6 +8298,8 @@
On other machines, condition code handling is different,
and specifying @code{"cc"} has no effect. But
it is valid no matter what the target.
+For platform-specific uses of flags, see also
+@ref{FlagOutputOperands,Flag Output Operands}.
@item "memory"
The @code{"memory"} clobber tells the compiler that the assembly code
