Re: [patch, fortran] PR77828 Linking gfortran-7 compiled program with libgfortran of 5.x allowed but crashes
On Mon, Oct 24, 2016 at 7:15 PM, Jerry DeLisle wrote: > On 10/24/2016 07:49 AM, Janne Blomqvist wrote: >> >> On Mon, Oct 24, 2016 at 2:53 PM, Jerry DeLisle >> wrote: >>> >>> On 10/18/2016 04:48 PM, Jerry DeLisle wrote: Hi Folks, The attached patch does some minor cleanup and bumps the libgfortran version number. I have wanted to reorder the dtp structure for many years now. Not strictly needed but it has bugged me forever. The bump is needed because of the significant changes from implementation of DTIO. >>> >>> --- snip --- >>> >>> Revised patch. Changed a single byte char to int for better memory >>> alignment >>> on 32 bit boundaries. Regression tested on x86-64-linux, FreeBSD, and >>> Power8. >>> >>> I plan to commit this to trunk sometime today. >> >> >> I understood from https://gcc.gnu.org/ml/fortran/2016-10/msg00168.html >> that fortran-dev won't be in shape for merging until GCC 8 stage 1. If >> so, shouldn't we avoid breaking the ABI until then? >> > > The ABI is already changed with the UD-DTIO changes, so we need to bump > version to avoid surprises with other earlier versions. Ugh. Is is possible to un-break the ABI then, or is the plan to merge fortran-dev regardless and fix the performance issues later? Seems to me that breaking the ABI for GCC 7 and again for GCC 8 is the worst choice. -- Janne Blomqvist
Re: RFC [1/3] divmod transform v2
On Sun, Oct 16, 2016 at 7:59 AM, Prathamesh Kulkarni
wrote:
> Hi,
> After approval from Bernd Schmidt, I committed the patch to remove
> optab functions for
> sdivmod_optab and udivmod_optab in optabs.def, which removes the block
> for divmod patch.
>
> This patch is mostly the same as previous one, except it drops
> targeting __udivmoddi4() because
> it gave undefined reference link error for calling __udivmoddi4() on
> aarch64-linux-gnu.
> It appears aarch64 has hardware insn for DImode div, so __udivmoddi4()
> isn't needed for the target
> (it was a bug in my patch that called __udivmoddi4() even though
> aarch64 supported hardware div).
>
> However this makes me wonder if it's guaranteed that __udivmoddi4()
> will be available for a target if it doesn't have hardware div and
> divmod insn and doesn't have target-specific libfunc for
> DImode divmod ? To be conservative, the attached patch doesn't
> generate call to __udivmoddi4.
>
> Passes bootstrap+test on x86_64-unknown-linux.
> Cross-tested on arm*-*-*, aarch64*-*-*.
> Verified that there are no regressions with SPEC2006 on
> x86_64-unknown-linux-gnu.
> OK to commit ?
I think the searching is still somewhat wrong - it's been some time
since my last look at the
patch so maybe I've said this already. Please bail out early for
stmt_can_throw_internal (stmt),
otherwise the top stmt search might end up not working. So
+
+ if (top_stmt == stmt && stmt_can_throw_internal (top_stmt))
+return false;
can go.
top_stmt may end up as a TRUNC_DIV_EXPR so it's pointless to only look
for another
TRUNC_DIV_EXPR later ... you may end up without a single TRUNC_MOD_EXPR.
Which means you want a div_seen and a mod_seen, or simply record the top_stmt
code and look for the opposite in the 2nd loop.
+ switch (gimple_assign_rhs_code (use_stmt))
+ {
+ case TRUNC_DIV_EXPR:
+ new_rhs = fold_build1 (REALPART_EXPR, TREE_TYPE (op1), res);
+ break;
+
+ case TRUNC_MOD_EXPR:
+ new_rhs = fold_build1 (IMAGPART_EXPR, TREE_TYPE (op2), res);
+ break;
+
why type of op1 and type of op2 in the other case? Choose one for consistency.
+ if (maybe_clean_or_replace_eh_stmt (use_stmt, use_stmt))
+ cfg_changed = true;
as you are rejecting all internally throwing stmts this shouldn't be necessary.
The patch is ok with those changes.
Thanks,
Richard.
> Thanks,
> Prathamesh
Re: [patch] Fix PHI optimization issue with boolean types
> Is the change to pass wi::to_widest really necessary? I think it has a
> runtime penalty.
Yes, because there is no == operator for a (tree, int) pair. Is there a cheap
way to convert a tree back into a wide_int? wi::to_wide? Or a combination
with decompose? Otherwise you can use eq_p, but this means that all other
callers of fits_to_tree_p are affected:
template
bool
wi::fits_to_tree_p (const T &x, const_tree type)
{
/* Short-circuit boolean types since various transformations assume that
they can only take values 0 and 1. */
if (TREE_CODE (type) == BOOLEAN_TYPE)
return eq_p (x, 0) || eq_p (x, 1);
instead of just int_fits_type_p (but I don't really know if there is a penalty
associated with eq_p here).
--
Eric Botcazou
Re: [patch] Fix PHI optimization issue with boolean types
On Tue, Oct 25, 2016 at 10:17 AM, Eric Botcazou wrote:
>> Is the change to pass wi::to_widest really necessary? I think it has a
>> runtime penalty.
>
> Yes, because there is no == operator for a (tree, int) pair.
Ah, yes. But operator== simply maps to wi::eq_p, so ...
> Is there a cheap
> way to convert a tree back into a wide_int? wi::to_wide? Or a combination
> with decompose? Otherwise you can use eq_p, but this means that all other
> callers of fits_to_tree_p are affected:
>
> template
> bool
> wi::fits_to_tree_p (const T &x, const_tree type)
> {
> /* Short-circuit boolean types since various transformations assume that
> they can only take values 0 and 1. */
> if (TREE_CODE (type) == BOOLEAN_TYPE)
> return eq_p (x, 0) || eq_p (x, 1);
>
> instead of just int_fits_type_p (but I don't really know if there is a penalty
> associated with eq_p here).
... this variant is fine it doesn't have any extra cost (the to_widest
in int_fits_type_p has).
I'm not sure what you mean with "all other callers of fits_to_tree_p
are affected" - that was desired.
Thanks,
Richard.
> --
> Eric Botcazou
Re: [patch, fortran] PR77828 Linking gfortran-7 compiled program with libgfortran of 5.x allowed but crashes
Hi Janne, > Ugh. Is is possible to un-break the ABI then, or is the plan to merge > fortran-dev regardless and fix the performance issues later? Seems to > me that breaking the ABI for GCC 7 and again for GCC 8 is the worst > choice. The ABI is broken without bumping the version. Bumping the ABI-version is not breaking it. That's called progress. I don't get why you oppose progress in gfortran that vehemently. There is only limited manpower to progress gfortran, so we all should be happy, that things are addressed. Users can always choose to stick with an older version of gfortran when they don't want to recompile everything. When I get the commercial compilers right, then do they change their ABI-version with every major release. Why is what is good for them, bad for us? We are making progress; Slowly, but progress nevertheless. So please motivate folks to do work ... - Andre -- Andre Vehreschild * Email: vehre ad gmx dot de
Re: [PATCH v2][AArch64] Fix symbol offset limit
ping
From: Wilco Dijkstra
Sent: 12 September 2016 15:50
To: Richard Earnshaw; GCC Patches
Cc: nd
Subject: Re: [PATCH v2][AArch64] Fix symbol offset limit
Wilco wrote:
> The original example is from GCC itself, the fixed_regs array is small but
> due to
> optimization we can end up with &fixed_regs + 0x.
We could also check the bounds of each symbol if they exist, like the patch
below.
In aarch64_classify_symbol symbols are allowed full-range offsets on
relocations.
This means the offset can use all of the +/-4GB offset, leaving no offset
available
for the symbol itself. This results in relocation overflow and link-time errors
for simple expressions like &global_char + 0xff00.
To avoid this, limit the offset to +/-1GB so that the symbol needs to be within
a
3GB offset from its references. For the tiny code model use a 64KB offset,
allowing
most of the 1MB range for code/data between the symbol and its references.
For symbols with a defined size, limit the offset to be within the size of the
symbol.
ChangeLog:
2016-09-12 Wilco Dijkstra
gcc/
* config/aarch64/aarch64.c (aarch64_classify_symbol):
Apply reasonable limit to symbol offsets.
testsuite/
* gcc.target/aarch64/symbol-range.c (foo): Set new limit.
* gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index
385bd560fb12cd5d404e6ddb2f01edf1fe72d729..275a828ac9e6e9b8187380c1b602ffb1b2bcfb21
100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9351,6 +9351,8 @@ aarch64_classify_symbol (rtx x, rtx offset)
if (aarch64_tls_symbol_p (x))
return aarch64_classify_tls_symbol (x);
+ const_tree decl = SYMBOL_REF_DECL (x);
+
switch (aarch64_cmodel)
{
case AARCH64_CMODEL_TINY:
@@ -9359,25 +9361,45 @@ aarch64_classify_symbol (rtx x, rtx offset)
we have no way of knowing the address of symbol at compile time
so we can't accurately say if the distance between the PC and
symbol + offset is outside the addressible range of +/-1M in the
- TINY code model. So we rely on images not being greater than
- 1M and cap the offset at 1M and anything beyond 1M will have to
- be loaded using an alternative mechanism. Furthermore if the
- symbol is a weak reference to something that isn't known to
- resolve to a symbol in this module, then force to memory. */
+ TINY code model. So we limit the maximum offset to +/-64KB and
+ assume the offset to the symbol is not larger than +/-(1M - 64KB).
+ Furthermore force to memory if the symbol is a weak reference to
+ something that doesn't resolve to a symbol in this module. */
if ((SYMBOL_REF_WEAK (x)
&& !aarch64_symbol_binds_local_p (x))
- || INTVAL (offset) < -1048575 || INTVAL (offset) > 1048575)
+ || !IN_RANGE (INTVAL (offset), -0x1, 0x1))
return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (decl_size
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_shwi (decl_size)))
+ return SYMBOL_FORCE_TO_MEM;
+ }
+
return SYMBOL_TINY_ABSOLUTE;
case AARCH64_CMODEL_SMALL:
/* Same reasoning as the tiny code model, but the offset cap here is
- 4G. */
+ 1G, allowing +/-3G for the offset to the symbol. */
if ((SYMBOL_REF_WEAK (x)
&& !aarch64_symbol_binds_local_p (x))
- || !IN_RANGE (INTVAL (offset), HOST_WIDE_INT_C (-4294967263),
- HOST_WIDE_INT_C (4294967264)))
+ || !IN_RANGE (INTVAL (offset), -0x4000, 0x4000))
return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (decl_size
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_shwi (decl_size)))
+ return SYMBOL_FORCE_TO_MEM;
+ }
+
return SYMBOL_SMALL_ABSOLUTE;
case AARCH64_CMODEL_TINY_PIC:
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
index
d7e46b059e41f2672b3a1da5506fa8944e752e01..d399a3637ed834ddc4bb429594c4ec229b5c2ea8
100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
-int fixed_regs[0x0020
Re: [PATCH][AArch64 - v3] Simplify eh_return implementation
ping
From: Wilco Dijkstra
Sent: 02 September 2016 12:31
To: Ramana Radhakrishnan; GCC Patches
Cc: nd
Subject: Re: [PATCH][AArch64 - v3] Simplify eh_return implementation
Ramana Radhakrishnan wrote:
> Can you please file a PR for this and add some testcases ? This sounds like
> a serious enough problem that needs to be looked at probably going back since
> the dawn of time.
I've created PR77455. Updated patch below:
This patch simplifies the handling of the EH return value. We force the use of
the
frame pointer so the return location is always at FP + 8. This means we can
emit
a simple volatile access in EH_RETURN_HANDLER_RTX without needing md
patterns, splitters and frame offset calculations. The new implementation also
fixes various bugs in aarch64_final_eh_return_addr, which does not work with
-fomit-frame-pointer, alloca or outgoing arguments.
Bootstrap OK, GCC Regression OK, OK for trunk? Would it be useful to backport
this to GCC6.x?
ChangeLog:
2016-09-02 Wilco Dijkstra
PR77455
gcc/
* config/aarch64/aarch64.md (eh_return): Remove pattern and splitter.
* config/aarch64/aarch64.h (AARCH64_EH_STACKADJ_REGNUM): Remove.
(EH_RETURN_HANDLER_RTX): New define.
* config/aarch64/aarch64.c (aarch64_frame_pointer_required):
Force frame pointer in EH return functions.
(aarch64_expand_epilogue): Add barrier for eh_return.
(aarch64_final_eh_return_addr): Remove.
(aarch64_eh_return_handler_rtx): New function.
* config/aarch64/aarch64-protos.h (aarch64_final_eh_return_addr):
Remove.
(aarch64_eh_return_handler_rtx): New prototype.
testsuite/
* gcc.target/aarch64/eh_return.c: New test.
--
diff --git a/gcc/config/aarch64/aarch64-protos.h
b/gcc/config/aarch64/aarch64-protos.h
index
3cdd69b8af1089a839e5d45cda94bc70a15cd777..327c0a97f6f687604afef249b79ac22628418070
100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -358,7 +358,7 @@ int aarch64_hard_regno_mode_ok (unsigned, machine_mode);
int aarch64_hard_regno_nregs (unsigned, machine_mode);
int aarch64_uxt_size (int, HOST_WIDE_INT);
int aarch64_vec_fpconst_pow_of_2 (rtx);
-rtx aarch64_final_eh_return_addr (void);
+rtx aarch64_eh_return_handler_rtx (void);
rtx aarch64_mask_from_zextract_ops (rtx, rtx);
const char *aarch64_output_move_struct (rtx *operands);
rtx aarch64_return_addr (int, rtx);
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index
003fec87e41db618570663f28cc2387a87e8252a..fa81e4b853daf08842955288861ec7e7acca
100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -400,9 +400,9 @@ extern unsigned aarch64_architecture_version;
#define ASM_DECLARE_FUNCTION_NAME(STR, NAME, DECL) \
aarch64_declare_function_name (STR, NAME, DECL)
-/* The register that holds the return address in exception handlers. */
-#define AARCH64_EH_STACKADJ_REGNUM (R0_REGNUM + 4)
-#define EH_RETURN_STACKADJ_RTX gen_rtx_REG (Pmode, AARCH64_EH_STACKADJ_REGNUM)
+/* For EH returns X4 contains the stack adjustment. */
+#define EH_RETURN_STACKADJ_RTX gen_rtx_REG (Pmode, R4_REGNUM)
+#define EH_RETURN_HANDLER_RTX aarch64_eh_return_handler_rtx ()
/* Don't use __builtin_setjmp until we've defined it. */
#undef DONT_USE_BUILTIN_SETJMP
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index
e742c19d76e6c62117aa62a990b9c2945aa06b74..f07d771ea343803e054e03f59c8c1efb698bf474
100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -2739,6 +2739,10 @@ aarch64_frame_pointer_required (void)
&& (!crtl->is_leaf || df_regs_ever_live_p (LR_REGNUM)))
return true;
+ /* Force a frame pointer for EH returns so the return address is at FP+8. */
+ if (crtl->calls_eh_return)
+ return true;
+
return false;
}
@@ -3298,7 +3302,8 @@ aarch64_expand_epilogue (bool for_sibcall)
+ cfun->machine->frame.saved_varargs_size) != 0;
/* Emit a barrier to prevent loads from a deallocated stack. */
- if (final_adjust > crtl->outgoing_args_size || cfun->calls_alloca)
+ if (final_adjust > crtl->outgoing_args_size || cfun->calls_alloca
+ || crtl->calls_eh_return)
{
emit_insn (gen_stack_tie (stack_pointer_rtx, stack_pointer_rtx));
need_barrier_p = false;
@@ -3366,52 +3371,15 @@ aarch64_expand_epilogue (bool for_sibcall)
emit_jump_insn (ret_rtx);
}
-/* Return the place to copy the exception unwinding return address to.
- This will probably be a stack slot, but could (in theory be the
- return register). */
+/* Implement EH_RETURN_HANDLER_RTX. The return address is stored at FP + 8.
+ The access needs to be volatile to prevent it from being removed. */
rtx
-aarch64_final_eh_return_addr (void)
+aarch64_eh_return_handler_rtx (void)
{
- HOST_WIDE_INT fp_offset;
-
- aarch64_layout_frame ();
-
- fp_offset = cfun->machine->frame.frame_size
-
Re: [PATCH][ARM] Avoid partial overlaps in DImode shifts
On 24/10/16 15:28, Wilco Dijkstra wrote:
> With -fpu=neon DI mode shifts are expanded after reload. DI mode registers
> can
> either fully or partially overlap. However the shift expansion code can only
> deal
> with the full overlap case, and generates incorrect code for partial overlaps.
> The fix is to add new variants that support either full overlap or no overlap.
>
> Bootstrap & regress on arm-linux-gnueabihf OK.
>
> This will need backporting to all active branches.
>
> ChangeLog:
> 2016-10-20 Wilco Dijkstra
>
> gcc/
> PR target/78041
> * config/arm/neon.md (ashldi3_neon): Add "r 0 i" and "&r r i" variants.
> Remove partial overlap check for shift by 1.
> (ashldi3_neon): Likewise.
> testsuite/
> * gcc.target/arm/pr78041.c: New test.
>
OK.
R.
> --
> diff --git a/gcc/config/arm/neon.md b/gcc/config/arm/neon.md
> index
> 05323334ffd81aeff33ee407b96c788d123b3fe3..59316de004107913c1db0951ced4d584999fc099
> 100644
> --- a/gcc/config/arm/neon.md
> +++ b/gcc/config/arm/neon.md
> @@ -1143,12 +1143,12 @@
> )
>
> (define_insn_and_split "ashldi3_neon"
> - [(set (match_operand:DI 0 "s_register_operand" "= w, w,?&r,?r,
> ?w,w")
> - (ashift:DI (match_operand:DI 1 "s_register_operand" " 0w, w, 0r, r,
> 0w,w")
> -(match_operand:SI 2 "general_operand""rUm, i, r,
> i,rUm,i")))
> - (clobber (match_scratch:SI 3 "= X,
> X,?&r, X, X,X"))
> - (clobber (match_scratch:SI 4 "= X,
> X,?&r, X, X,X"))
> - (clobber (match_scratch:DI 5 "=&w, X,
> X, X, &w,X"))
> + [(set (match_operand:DI 0 "s_register_operand" "= w, w,?&r,?r,?&r,
> ?w,w")
> + (ashift:DI (match_operand:DI 1 "s_register_operand" " 0w, w, 0r, 0, r,
> 0w,w")
> +(match_operand:SI 2 "general_operand""rUm, i, r, i,
> i,rUm,i")))
> + (clobber (match_scratch:SI 3 "= X,
> X,?&r, X, X, X,X"))
> + (clobber (match_scratch:SI 4 "= X,
> X,?&r, X, X, X,X"))
> + (clobber (match_scratch:DI 5 "=&w, X,
> X, X, X, &w,X"))
> (clobber (reg:CC_C CC_REGNUM))]
>"TARGET_NEON"
>"#"
> @@ -1180,9 +1180,11 @@
>}
> else
>{
> - if (operands[2] == CONST1_RTX (SImode)
> - && (!reg_overlap_mentioned_p (operands[0], operands[1])
> - || REGNO (operands[0]) == REGNO (operands[1])))
> + /* The shift expanders support either full overlap or no overlap. */
> + gcc_assert (!reg_overlap_mentioned_p (operands[0], operands[1])
> + || REGNO (operands[0]) == REGNO (operands[1]));
> +
> + if (operands[2] == CONST1_RTX (SImode))
> /* This clobbers CC. */
> emit_insn (gen_arm_ashldi3_1bit (operands[0], operands[1]));
> else
> @@ -1191,8 +1193,8 @@
>}
> DONE;
>}"
> - [(set_attr "arch"
> "neon_for_64bits,neon_for_64bits,*,*,avoid_neon_for_64bits,avoid_neon_for_64bits")
> - (set_attr "opt" "*,*,speed,speed,*,*")
> + [(set_attr "arch"
> "neon_for_64bits,neon_for_64bits,*,*,*,avoid_neon_for_64bits,avoid_neon_for_64bits")
> + (set_attr "opt" "*,*,speed,speed,speed,*,*")
> (set_attr "type" "multiple")]
> )
>
> @@ -1241,12 +1243,12 @@
> ;; ashrdi3_neon
> ;; lshrdi3_neon
> (define_insn_and_split "di3_neon"
> - [(set (match_operand:DI 0 "s_register_operand" "= w,
> w,?&r,?r,?w,?w")
> - (RSHIFTS:DI (match_operand:DI 1 "s_register_operand" " 0w, w, 0r, r,0w,
> w")
> - (match_operand:SI 2 "reg_or_int_operand" " r, i, r, i, r,
> i")))
> - (clobber (match_scratch:SI 3 "=2r, X,
> &r, X,2r, X"))
> - (clobber (match_scratch:SI 4 "= X, X,
> &r, X, X, X"))
> - (clobber (match_scratch:DI 5 "=&w, X,
> X, X,&w, X"))
> + [(set (match_operand:DI 0 "s_register_operand" "= w,
> w,?&r,?r,?&r,?w,?w")
> + (RSHIFTS:DI (match_operand:DI 1 "s_register_operand" " 0w, w, 0r, 0,
> r,0w, w")
> + (match_operand:SI 2 "reg_or_int_operand" " r, i, r, i,
> i, r, i")))
> + (clobber (match_scratch:SI 3 "=2r, X,
> &r, X, X,2r, X"))
> + (clobber (match_scratch:SI 4 "= X, X,
> &r, X, X, X, X"))
> + (clobber (match_scratch:DI 5 "=&w, X,
> X, X, X,&w, X"))
> (clobber (reg:CC CC_REGNUM))]
>"TARGET_NEON"
>"#"
> @@ -1282,9 +1284,11 @@
>}
> else
>{
> - if (operands[2] == CONST1_RTX (SImode)
> - && (!reg_overlap_mentioned_p (operands[0], operands[1])
> - || REGNO (operands[0]) == REGNO (operands[1])))
> + /* The shift expanders support either full overlap or no overlap. */
> + gcc_assert (!reg_overlap_mentioned_p (
Re: [rs6000] Add support for signed overflow arithmetic
> It is nicely generic as well, but requires more insns than this if your > ISA does not have a full complement of logical ops. Well, just an > "andnot" is enough, you don't actually need eqv here. Indeed, and it's rather spectacular for 64-bit operations on 32-bit machines because the sign bit trick can be done solely on the upper word, whereas a fully-fledged store-flag sequence is heavyweight in this case. Here's what the patched generic code yields at -O2: op__add32: add 10,3,4 xor 9,10,4 eqv 4,3,4 and. 8,9,4 blt- mr 3,10 blr op__add64: addc 4,4,6 adde 10,3,5 xor 9,10,5 eqv 5,3,5 and. 8,9,5 blt- mr 3,10 blr This should help 32-bit x86 too, which doesn't have 64-bit operations AFAICS. Thanks for the tip, I'll submit the change to the generic code separately. -- Eric Botcazou
Re: [patch] [gsoc] [gimplefe] GIMPLE FE Project
On Thu, Oct 6, 2016 at 1:28 AM, Joseph Myers wrote:
> On Fri, 26 Aug 2016, Prasad Ghangal wrote:
>
>> >> Thanks for your feedback. I had missed removing some unwanted code
>> >> while code cleanup. I have updated the patch.
>> >> I am not sure if we should move all gimple parsing related functions
>> >> to the new file (?)
>> >
>> > I think it might be good to make the parts of the C parser you use more
>> > obvious (you'd need to export functions like c_parser_next_token_is).
>> >
>> > The easiest way to "force" that is to put all of the gimple parsing into
>> > a separate file.
>> >
>> > Note I am not so much concerned about this at the moment, the parts to
>> > improve would be avoiding more of the C-isms like convert_lvalue_to_rvalue,
>> > handling of SIZEOF_EXPR and other stuff that looks redundant (you've
>> > probably copied this from the C parsing routines and refactored it).
>> > Also the GIMPLE parser shouldn't do any warnings (just spotted
>> > a call to warn_for_memset).
>> >
>> PFA updated patch (successfully bootstrapped and tested on
>> x86_64-pc-linux-gnu). I have removed unnecessary code. On side I am
>> also trying to move gimple parser related functions to new file. But
>> for it we also have to move structs like c_token, c_parser. Won't it
>> disturb the c-parser code structure ?
Thanks Joseph for the review. Prasad - do you have time in the next few weeks
to continue working on this? I'm currently trying to move what is on the github
branch to a branch on git://gcc.gnu.org/git/gcc.git to make it mergeable
(looks like the github repo isn't a clone of the gcc git mirror on github?).
Thanks,
Richard.
> I think the GIMPLE parsing should go in a separate file (meaning exporting
> relevant types and function declarations in a new c-parser.h).
>
>> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
>> index a5358ed..3c4d2cc 100644
>> --- a/gcc/c-family/c.opt
>> +++ b/gcc/c-family/c.opt
>> @@ -200,6 +200,10 @@ F
>> Driver C ObjC C++ ObjC++ Joined Separate MissingArgError(missing path after
>> %qs)
>> -F Add to the end of the main framework include path.
>>
>> +fgimple
>> +C Var(flag_gimple) Init(0)
>> +Enable parsing GIMPLE
>
> You should get a test failure here from the missing "." at the end of the
> help text.
>
> Of course the option also needs documenting in invoke.texi.
>
>> @@ -1659,6 +1695,9 @@ c_parser_declaration_or_fndef (c_parser *parser, bool
>> fndef_ok,
>>tree all_prefix_attrs;
>>bool diagnosed_no_specs = false;
>>location_t here = c_parser_peek_token (parser)->location;
>> + bool gimple_body_p = false;
>> + opt_pass *pass = NULL;
>> + bool startwith_p = false;
>
> The comment above the function needs updating to document the new syntax.
>
>> +static void
>> +c_parser_gimple_expression (c_parser *parser, gimple_seq *seq)
>> +{
>> + struct c_expr lhs, rhs;
>> + gimple *assign = NULL;
>> + enum tree_code subcode = NOP_EXPR;
>> + location_t loc;
>> + tree arg = NULL_TREE;
>> + auto_vec vargs;
>> +
>> + lhs = c_parser_gimple_unary_expression (parser);
>> + rhs.value = error_mark_node;
>> +
>> + if (c_parser_next_token_is (parser, CPP_EQ))
>> +{
>> + c_parser_consume_token (parser);
>> +}
>
> Redundant braces around a single statement. Also, this looks wrong, in
> that it seems like you'd accept a random '=' token at this point
> regardless of what follows and whether '=' makes sense in this context.
> You need to have proper cases: if '=' parse what makes sense after '=',
> otherwise parse what makes sense without '=', so that invalid syntax is
> not accepted.
>
>> + if (c_parser_next_token_is (parser, CPP_AND) ||
>> + c_parser_next_token_is (parser, CPP_MULT) ||
>> + c_parser_next_token_is (parser, CPP_PLUS) ||
>> + c_parser_next_token_is (parser, CPP_MINUS) ||
>> + c_parser_next_token_is (parser, CPP_COMPL) ||
>> + c_parser_next_token_is (parser, CPP_NOT))
>
> Operators go at the start of a continuation line, not at the end of the
> line.
>
>> + if (!c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>"))
>> + {
>> + return;
>> + }
>
> Please generally review the patch for redundant braces and remove them.
>
>> + /* ssa token string. */
>> + const char *ssa_token = IDENTIFIER_POINTER (c_parser_peek_token
>> (parser)->value);
>> + token = new char [strlen (ssa_token)];
>> + strcpy (token, ssa_token);
>
> That looks like a buffer overrun. To copy a string ssa_token, you need
> strlen (ssa_token) + 1 bytes of space.
>
>> + /* seperate var name and version. */
>
> Uppercase letters at start of comments, throughout the patch (and it
> should be "Separate", with 'a' not 'e').
>
> --
> Joseph S. Myers
> jos...@codesourcery.com
Re: [patch] Fix PHI optimization issue with boolean types
> I'm not sure what you mean with "all other callers of fits_to_tree_p > are affected" - that was desired. Affected by the from == to eq_p change: + if (TREE_CODE (type) == BOOLEAN_TYPE) +return x == 0 || x == 1; vs + if (TREE_CODE (type) == BOOLEAN_TYPE) +return eq_p (x, 0) || eq_p (x, 1); Is eq_p free for them too? If no, then you distribute the overhead over all the callers instead of just int_fits_type_p (for which the call is unlikely). -- Eric Botcazou
Re: [patch] Fix PHI optimization issue with boolean types
On Tue, Oct 25, 2016 at 12:22 PM, Eric Botcazou wrote: >> I'm not sure what you mean with "all other callers of fits_to_tree_p >> are affected" - that was desired. > > Affected by the from == to eq_p change: > > + if (TREE_CODE (type) == BOOLEAN_TYPE) > +return x == 0 || x == 1; > > vs > > + if (TREE_CODE (type) == BOOLEAN_TYPE) > +return eq_p (x, 0) || eq_p (x, 1); > > Is eq_p free for them too? If no, then you distribute the overhead over all > the callers instead of just int_fits_type_p (for which the call is unlikely). Yes, eq_p is equivalent to == (apart from the overloading issue you ran into). Richard. > -- > Eric Botcazou
Re: RFC [1/3] divmod transform v2
On 25 October 2016 at 13:43, Richard Biener wrote:
> On Sun, Oct 16, 2016 at 7:59 AM, Prathamesh Kulkarni
> wrote:
>> Hi,
>> After approval from Bernd Schmidt, I committed the patch to remove
>> optab functions for
>> sdivmod_optab and udivmod_optab in optabs.def, which removes the block
>> for divmod patch.
>>
>> This patch is mostly the same as previous one, except it drops
>> targeting __udivmoddi4() because
>> it gave undefined reference link error for calling __udivmoddi4() on
>> aarch64-linux-gnu.
>> It appears aarch64 has hardware insn for DImode div, so __udivmoddi4()
>> isn't needed for the target
>> (it was a bug in my patch that called __udivmoddi4() even though
>> aarch64 supported hardware div).
>>
>> However this makes me wonder if it's guaranteed that __udivmoddi4()
>> will be available for a target if it doesn't have hardware div and
>> divmod insn and doesn't have target-specific libfunc for
>> DImode divmod ? To be conservative, the attached patch doesn't
>> generate call to __udivmoddi4.
>>
>> Passes bootstrap+test on x86_64-unknown-linux.
>> Cross-tested on arm*-*-*, aarch64*-*-*.
>> Verified that there are no regressions with SPEC2006 on
>> x86_64-unknown-linux-gnu.
>> OK to commit ?
>
> I think the searching is still somewhat wrong - it's been some time
> since my last look at the
> patch so maybe I've said this already. Please bail out early for
> stmt_can_throw_internal (stmt),
> otherwise the top stmt search might end up not working. So
>
> +
> + if (top_stmt == stmt && stmt_can_throw_internal (top_stmt))
> +return false;
>
> can go.
>
> top_stmt may end up as a TRUNC_DIV_EXPR so it's pointless to only look
> for another
> TRUNC_DIV_EXPR later ... you may end up without a single TRUNC_MOD_EXPR.
> Which means you want a div_seen and a mod_seen, or simply record the top_stmt
> code and look for the opposite in the 2nd loop.
Um sorry I don't quite understand how we could end up without a trunc_mod stmt ?
The 2nd loop adds both trunc_div and trunc_mod to stmts vector, and
checks if we have
come across at least a single trunc_div stmt (and we bail out if no
div is seen).
At 2nd loop I suppose we don't need mod_seen, because stmt is
guaranteed to be trunc_mod_expr.
In the 2nd loop the following condition will never trigger for stmt:
if (stmt_can_throw_internal (use_stmt))
continue;
since we checked before hand if stmt could throw and chose to bail out
in that case.
and the following condition would also not trigger for stmt:
if (!dominated_by_p (CDI_DOMINATORS, gimple_bb (use_stmt), top_bb))
{
end_imm_use_stmt_traverse (&use_iter);
return false;
}
since gimple_bb (stmt) is always dominated by gimple_bb (top_stmt).
The case where top_stmt == stmt, we wouldn't reach the above
condition, since we have above it:
if (top_stmt == stmt)
continue;
So IIUC, top_stmt and stmt would always get added to stmts vector.
Am I missing something ?
Thanks,
Prathamesh
>
> + switch (gimple_assign_rhs_code (use_stmt))
> + {
> + case TRUNC_DIV_EXPR:
> + new_rhs = fold_build1 (REALPART_EXPR, TREE_TYPE (op1), res);
> + break;
> +
> + case TRUNC_MOD_EXPR:
> + new_rhs = fold_build1 (IMAGPART_EXPR, TREE_TYPE (op2), res);
> + break;
> +
>
> why type of op1 and type of op2 in the other case? Choose one for
> consistency.
>
> + if (maybe_clean_or_replace_eh_stmt (use_stmt, use_stmt))
> + cfg_changed = true;
>
> as you are rejecting all internally throwing stmts this shouldn't be
> necessary.
>
> The patch is ok with those changes.
>
> Thanks,
> Richard.
>
>
>> Thanks,
>> Prathamesh
[PATCH] combine: Improve change_zero_ext (fixes PR71847)
This improves a few things in change_zero_ext. Firstly, it should use
the passed in pattern in recog_for_combine, not the pattern of the insn
(they are not the same if the whole pattern was replaced). Secondly,
it handled zero_ext of a subreg, but with hard registers we do not get
a subreg, instead the mode of the reg is changed. So this handles that.
Thirdly, after changing a zero_ext to an AND, the resulting RTL may become
non-canonical, like (ior (ashift ..) (and ..)); the AND should be first,
it is commutative. And lastly, zero_extract as a set_dest wasn't handled
at all, but now it is.
This fixes the testcase in PR71847, and improves code generation in some
other edge cases too.
Tested on powerpc64-linux {-m32,-m64}; I'll also test it on x86 and
will build lots of crosses before committing.
Segher
2016-10-25 Segher Boessenkool
PR target/71847
* combine.c (change_zero_ext): Handle zero_ext of hard registers.
Swap commutative operands in new RTL if needed. Handle zero_ext
in the set_dest.
(recog_for_combine): Pass *pnewpat to change_zero_ext instead of
PATTERN (insn).
---
gcc/combine.c | 55 +++
1 file changed, 51 insertions(+), 4 deletions(-)
diff --git a/gcc/combine.c b/gcc/combine.c
index ee12714..b46405b 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -11140,9 +11140,10 @@ recog_for_combine_1 (rtx *pnewpat, rtx_insn *insn, rtx
*pnotes)
Return whether anything was so changed. */
static bool
-change_zero_ext (rtx *src)
+change_zero_ext (rtx pat)
{
bool changed = false;
+ rtx *src = &SET_SRC (pat);
subrtx_ptr_iterator::array_type array;
FOR_EACH_SUBRTX_PTR (iter, array, src, NONCONST)
@@ -11174,6 +11175,14 @@ change_zero_ext (rtx *src)
size = GET_MODE_PRECISION (GET_MODE (XEXP (x, 0)));
x = SUBREG_REG (XEXP (x, 0));
}
+ else if (GET_CODE (x) == ZERO_EXTEND
+ && SCALAR_INT_MODE_P (mode)
+ && REG_P (XEXP (x, 0))
+ && HARD_REGISTER_P (XEXP (x, 0)))
+ {
+ size = GET_MODE_PRECISION (GET_MODE (XEXP (x, 0)));
+ x = gen_rtx_REG (mode, REGNO (XEXP (x, 0)));
+ }
else
continue;
@@ -11184,6 +11193,44 @@ change_zero_ext (rtx *src)
changed = true;
}
+ if (changed)
+FOR_EACH_SUBRTX_PTR (iter, array, src, NONCONST)
+ {
+ rtx x = **iter;
+ if (COMMUTATIVE_ARITH_P (x)
+ && swap_commutative_operands_p (XEXP (x, 0), XEXP (x, 1)))
+ {
+ rtx tem = XEXP (x, 0);
+ SUBST (XEXP (x, 0), XEXP (x, 1));
+ SUBST (XEXP (x, 1), tem);
+ }
+ }
+
+ rtx *dst = &SET_DEST (pat);
+ if (GET_CODE (*dst) == ZERO_EXTRACT
+ && REG_P (XEXP (*dst, 0))
+ && CONST_INT_P (XEXP (*dst, 1))
+ && CONST_INT_P (XEXP (*dst, 2)))
+{
+ rtx reg = XEXP (*dst, 0);
+ int width = INTVAL (XEXP (*dst, 1));
+ int offset = INTVAL (XEXP (*dst, 2));
+ machine_mode mode = GET_MODE (reg);
+ int reg_width = GET_MODE_PRECISION (mode);
+ if (BITS_BIG_ENDIAN)
+ offset = reg_width - width - offset;
+
+ wide_int mask = wi::shifted_mask (offset, width, true, reg_width);
+ rtx x = gen_rtx_AND (mode, reg, immed_wide_int_const (mask, mode));
+ rtx y = simplify_gen_binary (ASHIFT, mode, SET_SRC (pat),
+ GEN_INT (offset));
+ rtx z = simplify_gen_binary (IOR, mode, x, y);
+ SUBST (SET_DEST (pat), reg);
+ SUBST (SET_SRC (pat), z);
+
+ changed = true;
+}
+
return changed;
}
@@ -11206,7 +11253,7 @@ change_zero_ext (rtx *src)
static int
recog_for_combine (rtx *pnewpat, rtx_insn *insn, rtx *pnotes)
{
- rtx pat = PATTERN (insn);
+ rtx pat = *pnewpat;
int insn_code_number = recog_for_combine_1 (pnewpat, insn, pnotes);
if (insn_code_number >= 0 || check_asm_operands (pat))
return insn_code_number;
@@ -11215,7 +11262,7 @@ recog_for_combine (rtx *pnewpat, rtx_insn *insn, rtx
*pnotes)
bool changed = false;
if (GET_CODE (pat) == SET)
-changed = change_zero_ext (&SET_SRC (pat));
+changed = change_zero_ext (pat);
else if (GET_CODE (pat) == PARALLEL)
{
int i;
@@ -11223,7 +11270,7 @@ recog_for_combine (rtx *pnewpat, rtx_insn *insn, rtx
*pnotes)
{
rtx set = XVECEXP (pat, 0, i);
if (GET_CODE (set) == SET)
- changed |= change_zero_ext (&SET_SRC (set));
+ changed |= change_zero_ext (set);
}
}
--
1.9.3
Re: RFC [1/3] divmod transform v2
On Tue, 25 Oct 2016, Prathamesh Kulkarni wrote:
> On 25 October 2016 at 13:43, Richard Biener
> wrote:
> > On Sun, Oct 16, 2016 at 7:59 AM, Prathamesh Kulkarni
> > wrote:
> >> Hi,
> >> After approval from Bernd Schmidt, I committed the patch to remove
> >> optab functions for
> >> sdivmod_optab and udivmod_optab in optabs.def, which removes the block
> >> for divmod patch.
> >>
> >> This patch is mostly the same as previous one, except it drops
> >> targeting __udivmoddi4() because
> >> it gave undefined reference link error for calling __udivmoddi4() on
> >> aarch64-linux-gnu.
> >> It appears aarch64 has hardware insn for DImode div, so __udivmoddi4()
> >> isn't needed for the target
> >> (it was a bug in my patch that called __udivmoddi4() even though
> >> aarch64 supported hardware div).
> >>
> >> However this makes me wonder if it's guaranteed that __udivmoddi4()
> >> will be available for a target if it doesn't have hardware div and
> >> divmod insn and doesn't have target-specific libfunc for
> >> DImode divmod ? To be conservative, the attached patch doesn't
> >> generate call to __udivmoddi4.
> >>
> >> Passes bootstrap+test on x86_64-unknown-linux.
> >> Cross-tested on arm*-*-*, aarch64*-*-*.
> >> Verified that there are no regressions with SPEC2006 on
> >> x86_64-unknown-linux-gnu.
> >> OK to commit ?
> >
> > I think the searching is still somewhat wrong - it's been some time
> > since my last look at the
> > patch so maybe I've said this already. Please bail out early for
> > stmt_can_throw_internal (stmt),
> > otherwise the top stmt search might end up not working. So
> >
> > +
> > + if (top_stmt == stmt && stmt_can_throw_internal (top_stmt))
> > +return false;
> >
> > can go.
> >
> > top_stmt may end up as a TRUNC_DIV_EXPR so it's pointless to only look
> > for another
> > TRUNC_DIV_EXPR later ... you may end up without a single TRUNC_MOD_EXPR.
> > Which means you want a div_seen and a mod_seen, or simply record the
> > top_stmt
> > code and look for the opposite in the 2nd loop.
> Um sorry I don't quite understand how we could end up without a trunc_mod
> stmt ?
> The 2nd loop adds both trunc_div and trunc_mod to stmts vector, and
> checks if we have
> come across at least a single trunc_div stmt (and we bail out if no
> div is seen).
>
> At 2nd loop I suppose we don't need mod_seen, because stmt is
> guaranteed to be trunc_mod_expr.
> In the 2nd loop the following condition will never trigger for stmt:
> if (stmt_can_throw_internal (use_stmt))
> continue;
> since we checked before hand if stmt could throw and chose to bail out
> in that case.
>
> and the following condition would also not trigger for stmt:
> if (!dominated_by_p (CDI_DOMINATORS, gimple_bb (use_stmt), top_bb))
> {
> end_imm_use_stmt_traverse (&use_iter);
> return false;
> }
> since gimple_bb (stmt) is always dominated by gimple_bb (top_stmt).
>
> The case where top_stmt == stmt, we wouldn't reach the above
> condition, since we have above it:
> if (top_stmt == stmt)
> continue;
>
> So IIUC, top_stmt and stmt would always get added to stmts vector.
> Am I missing something ?
Ah, indeed. Maybe add a comment then, it wasn't really obvious ;)
Please still move the stmt_can_throw_internal (stmt) check up.
Thanks,
Richard.
> Thanks,
> Prathamesh
> >
> > + switch (gimple_assign_rhs_code (use_stmt))
> > + {
> > + case TRUNC_DIV_EXPR:
> > + new_rhs = fold_build1 (REALPART_EXPR, TREE_TYPE (op1), res);
> > + break;
> > +
> > + case TRUNC_MOD_EXPR:
> > + new_rhs = fold_build1 (IMAGPART_EXPR, TREE_TYPE (op2), res);
> > + break;
> > +
> >
> > why type of op1 and type of op2 in the other case? Choose one for
> > consistency.
> >
> > + if (maybe_clean_or_replace_eh_stmt (use_stmt, use_stmt))
> > + cfg_changed = true;
> >
> > as you are rejecting all internally throwing stmts this shouldn't be
> > necessary.
> >
> > The patch is ok with those changes.
> >
> > Thanks,
> > Richard.
> >
> >
> >> Thanks,
> >> Prathamesh
>
>
--
Richard Biener
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
Re: [patch] [gsoc] [gimplefe] GIMPLE FE Project
On 25 October 2016 at 15:49, Richard Biener wrote:
> On Thu, Oct 6, 2016 at 1:28 AM, Joseph Myers wrote:
>> On Fri, 26 Aug 2016, Prasad Ghangal wrote:
>>
>>> >> Thanks for your feedback. I had missed removing some unwanted code
>>> >> while code cleanup. I have updated the patch.
>>> >> I am not sure if we should move all gimple parsing related functions
>>> >> to the new file (?)
>>> >
>>> > I think it might be good to make the parts of the C parser you use more
>>> > obvious (you'd need to export functions like c_parser_next_token_is).
>>> >
>>> > The easiest way to "force" that is to put all of the gimple parsing into
>>> > a separate file.
>>> >
>>> > Note I am not so much concerned about this at the moment, the parts to
>>> > improve would be avoiding more of the C-isms like
>>> > convert_lvalue_to_rvalue,
>>> > handling of SIZEOF_EXPR and other stuff that looks redundant (you've
>>> > probably copied this from the C parsing routines and refactored it).
>>> > Also the GIMPLE parser shouldn't do any warnings (just spotted
>>> > a call to warn_for_memset).
>>> >
>>> PFA updated patch (successfully bootstrapped and tested on
>>> x86_64-pc-linux-gnu). I have removed unnecessary code. On side I am
>>> also trying to move gimple parser related functions to new file. But
>>> for it we also have to move structs like c_token, c_parser. Won't it
>>> disturb the c-parser code structure ?
>
> Thanks Joseph for the review. Prasad - do you have time in the next few weeks
> to continue working on this? I'm currently trying to move what is on the
> github
> branch to a branch on git://gcc.gnu.org/git/gcc.git to make it mergeable
Sorry I couldn't work on the project in last few weeks. Since I will
be on vacation in the next week, I will definitely work on it. If we
can't merge it before closure of stage1, can we merge it in the stage2
or stage3?
> (looks like the github repo isn't a clone of the gcc git mirror on github?).
No. (Unfortunately) I had reinitialised git locally. That's why I am
also struggling while rebasing and syncing to the trunk. Any solution?
Since I am employed now, do I need to update the copyright assignment?
Thanks,
Prasad
>
> Thanks,
> Richard.
>
>> I think the GIMPLE parsing should go in a separate file (meaning exporting
>> relevant types and function declarations in a new c-parser.h).
>>
>>> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
>>> index a5358ed..3c4d2cc 100644
>>> --- a/gcc/c-family/c.opt
>>> +++ b/gcc/c-family/c.opt
>>> @@ -200,6 +200,10 @@ F
>>> Driver C ObjC C++ ObjC++ Joined Separate MissingArgError(missing path
>>> after %qs)
>>> -F Add to the end of the main framework include path.
>>>
>>> +fgimple
>>> +C Var(flag_gimple) Init(0)
>>> +Enable parsing GIMPLE
>>
>> You should get a test failure here from the missing "." at the end of the
>> help text.
>>
>> Of course the option also needs documenting in invoke.texi.
>>
>>> @@ -1659,6 +1695,9 @@ c_parser_declaration_or_fndef (c_parser *parser, bool
>>> fndef_ok,
>>>tree all_prefix_attrs;
>>>bool diagnosed_no_specs = false;
>>>location_t here = c_parser_peek_token (parser)->location;
>>> + bool gimple_body_p = false;
>>> + opt_pass *pass = NULL;
>>> + bool startwith_p = false;
>>
>> The comment above the function needs updating to document the new syntax.
>>
>>> +static void
>>> +c_parser_gimple_expression (c_parser *parser, gimple_seq *seq)
>>> +{
>>> + struct c_expr lhs, rhs;
>>> + gimple *assign = NULL;
>>> + enum tree_code subcode = NOP_EXPR;
>>> + location_t loc;
>>> + tree arg = NULL_TREE;
>>> + auto_vec vargs;
>>> +
>>> + lhs = c_parser_gimple_unary_expression (parser);
>>> + rhs.value = error_mark_node;
>>> +
>>> + if (c_parser_next_token_is (parser, CPP_EQ))
>>> +{
>>> + c_parser_consume_token (parser);
>>> +}
>>
>> Redundant braces around a single statement. Also, this looks wrong, in
>> that it seems like you'd accept a random '=' token at this point
>> regardless of what follows and whether '=' makes sense in this context.
>> You need to have proper cases: if '=' parse what makes sense after '=',
>> otherwise parse what makes sense without '=', so that invalid syntax is
>> not accepted.
>>
>>> + if (c_parser_next_token_is (parser, CPP_AND) ||
>>> + c_parser_next_token_is (parser, CPP_MULT) ||
>>> + c_parser_next_token_is (parser, CPP_PLUS) ||
>>> + c_parser_next_token_is (parser, CPP_MINUS) ||
>>> + c_parser_next_token_is (parser, CPP_COMPL) ||
>>> + c_parser_next_token_is (parser, CPP_NOT))
>>
>> Operators go at the start of a continuation line, not at the end of the
>> line.
>>
>>> + if (!c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>"))
>>> + {
>>> + return;
>>> + }
>>
>> Please generally review the patch for redundant braces and remove them.
>>
>>> + /* ssa token string. */
>>> + const char *ssa_token = IDENTIFIER_POINTER (c_parser_peek_token
>>> (parser)->value);
>>> + token = new char [strle
Re: [patch] [gsoc] [gimplefe] GIMPLE FE Project
On Tue, Oct 25, 2016 at 1:13 PM, Prasad Ghangal
wrote:
> On 25 October 2016 at 15:49, Richard Biener
> wrote:
>> On Thu, Oct 6, 2016 at 1:28 AM, Joseph Myers wrote:
>>> On Fri, 26 Aug 2016, Prasad Ghangal wrote:
>>>
>> Thanks for your feedback. I had missed removing some unwanted code
>> while code cleanup. I have updated the patch.
>> I am not sure if we should move all gimple parsing related functions
>> to the new file (?)
>
> I think it might be good to make the parts of the C parser you use more
> obvious (you'd need to export functions like c_parser_next_token_is).
>
> The easiest way to "force" that is to put all of the gimple parsing into
> a separate file.
>
> Note I am not so much concerned about this at the moment, the parts to
> improve would be avoiding more of the C-isms like
> convert_lvalue_to_rvalue,
> handling of SIZEOF_EXPR and other stuff that looks redundant (you've
> probably copied this from the C parsing routines and refactored it).
> Also the GIMPLE parser shouldn't do any warnings (just spotted
> a call to warn_for_memset).
>
PFA updated patch (successfully bootstrapped and tested on
x86_64-pc-linux-gnu). I have removed unnecessary code. On side I am
also trying to move gimple parser related functions to new file. But
for it we also have to move structs like c_token, c_parser. Won't it
disturb the c-parser code structure ?
>>
>> Thanks Joseph for the review. Prasad - do you have time in the next few
>> weeks
>> to continue working on this? I'm currently trying to move what is on the
>> github
>> branch to a branch on git://gcc.gnu.org/git/gcc.git to make it mergeable
> Sorry I couldn't work on the project in last few weeks. Since I will
> be on vacation in the next week, I will definitely work on it. If we
> can't merge it before closure of stage1, can we merge it in the stage2
> or stage3?
I think we might be able to merge early during stage3 as well. I'm working my
way through the changes that affect not just the GIMPLE FE itself.
>> (looks like the github repo isn't a clone of the gcc git mirror on github?).
> No. (Unfortunately) I had reinitialised git locally. That's why I am
> also struggling while rebasing and syncing to the trunk. Any solution?
I almost finished pushing the last state plus Josephs review comments
fixed (the style ones) to the official GCC git mirror as a branch off
current trunk.
I'll send a short announcement once I managed to "push" ...
> Since I am employed now, do I need to update the copyright assignment?
If your employer has a copyright assignment then things should be fine.
If you work on this during non-work time then your personal assignment is
also fine.
Thanks,
Richard.
>
> Thanks,
> Prasad
>>
>> Thanks,
>> Richard.
>>
>>> I think the GIMPLE parsing should go in a separate file (meaning exporting
>>> relevant types and function declarations in a new c-parser.h).
>>>
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index a5358ed..3c4d2cc 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -200,6 +200,10 @@ F
Driver C ObjC C++ ObjC++ Joined Separate MissingArgError(missing path
after %qs)
-F Add to the end of the main framework include path.
+fgimple
+C Var(flag_gimple) Init(0)
+Enable parsing GIMPLE
>>>
>>> You should get a test failure here from the missing "." at the end of the
>>> help text.
>>>
>>> Of course the option also needs documenting in invoke.texi.
>>>
@@ -1659,6 +1695,9 @@ c_parser_declaration_or_fndef (c_parser *parser,
bool fndef_ok,
tree all_prefix_attrs;
bool diagnosed_no_specs = false;
location_t here = c_parser_peek_token (parser)->location;
+ bool gimple_body_p = false;
+ opt_pass *pass = NULL;
+ bool startwith_p = false;
>>>
>>> The comment above the function needs updating to document the new syntax.
>>>
+static void
+c_parser_gimple_expression (c_parser *parser, gimple_seq *seq)
+{
+ struct c_expr lhs, rhs;
+ gimple *assign = NULL;
+ enum tree_code subcode = NOP_EXPR;
+ location_t loc;
+ tree arg = NULL_TREE;
+ auto_vec vargs;
+
+ lhs = c_parser_gimple_unary_expression (parser);
+ rhs.value = error_mark_node;
+
+ if (c_parser_next_token_is (parser, CPP_EQ))
+{
+ c_parser_consume_token (parser);
+}
>>>
>>> Redundant braces around a single statement. Also, this looks wrong, in
>>> that it seems like you'd accept a random '=' token at this point
>>> regardless of what follows and whether '=' makes sense in this context.
>>> You need to have proper cases: if '=' parse what makes sense after '=',
>>> otherwise parse what makes sense without '=', so that invalid syntax is
>>> not accepted.
>>>
+ if (c_parser_next_token_is (parser, CPP_AND) ||
+
[PATCH GCC][1/4]Simplify (convert1 (minmax ((convert2 (x) c)))) into minmax (x c)
Hi,
This is a patch set adding various match.pd patterns in order to generate
simplified MIN/MAX_EXPR mostly from COND_EXPR. This is the first one
optimizing (convert1 (minmax ((convert2 (x) c to minmax (x c), if convert2
promotes x and convert1 demotes back to x's type. With this patch, generated
assembly for test:
.L4:
ldr q0, [x3, x1]
add w2, w2, 1
cmp w0, w2
ushll v2.4s, v0.4h, 0
ushll2 v1.4s, v0.8h, 0
uminv2.4s, v2.4s, v3.4s
uminv1.4s, v1.4s, v3.4s
xtn v4.4h, v2.4s
xtn2v4.8h, v1.4s
str q4, [x3, x1]
add x1, x1, 16
bhi .L4
can be improved to:
.L4:
ldr q0, [x3, x1]
add w2, w2, 1
cmp w0, w2
uminv1.8h, v0.8h, v2.8h
str q1, [x3, x1]
add x1, x1, 16
bhi .L4
Bootstrap and test on x86_64 and AArch64 for whole patch set. Is it OK?
Thanks,
bin
2016-10-21 Bin Cheng
* match.pd ((convert1 (minmax ((convert2 (x) c -> minmax (x c)):
New pattern.
gcc/testsuite/ChangeLog
2016-10-21 Bin Cheng
* gcc.dg/fold-convmaxconv-1.c: New test.
* gcc.dg/fold-convminconv-1.c: New test.diff --git a/gcc/match.pd b/gcc/match.pd
index b782a1e..7365bc1 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -1331,6 +1331,28 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
&& TYPE_MIN_VALUE (type)
&& operand_equal_p (@1, TYPE_MIN_VALUE (type), OEP_ONLY_CONST))
@0)))
+
+#if GIMPLE
+/* (convert1 (minmax ((convert2 (x) c -> minmax (x c) if convert2
+ promotes x and convert1 demotes back to x's type. */
+
+(for minmax (min max)
+ (simplify
+ (convert (minmax@0 (convert @1) INTEGER_CST@2))
+ (if (types_compatible_p (TREE_TYPE (@1), type))
+ (with
+{
+ tree minmax_type = TREE_TYPE (@0);
+ signop sgn = TYPE_SIGN (type);
+ widest_int type_min = widest_int::from (wi::min_value (type), sgn);
+ widest_int type_max = widest_int::from (wi::max_value (type), sgn);
+}
+(if (sgn == TYPE_SIGN (minmax_type)
+&& TYPE_PRECISION (minmax_type) >= TYPE_PRECISION (type)
+&& wi::to_widest (@2) >= type_min && wi::to_widest (@2) <= type_max)
+ (minmax @1 { fold_convert (type, @2); }))
+#endif
+
(for minmax (FMIN FMAX)
/* If either argument is NaN, return the other one. Avoid the
transformation if we get (and honor) a signalling NaN. */
diff --git a/gcc/testsuite/gcc.dg/fold-convmaxconv-1.c
b/gcc/testsuite/gcc.dg/fold-convmaxconv-1.c
new file mode 100644
index 000..38b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/fold-convmaxconv-1.c
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fdump-tree-optimized" } */
+
+int foo (short a[], int x)
+{
+ unsigned int i;
+ for (i = 0; i < 1000; i++)
+{
+ x = a[i];
+ a[i] = (x <= 0 ? 0 : x);
+}
+ return x;
+}
+
+/* { dg-final { scan-tree-dump-not " = MAX_EXPR = 255 ? 255 : x);
+}
+ return x;
+}
+
+/* { dg-final { scan-tree-dump-not " = MIN_EXPR
[PATCH GCC][3/4]Add support for constant operand in pattern (convert (op:s (convert@2 @0) (convert?@3 @1)))
Hi,
This is an update patch for
https://gcc.gnu.org/ml/gcc-patches/2016-10/msg00738.html . In this version,
existing pattern (convert (op:s (convert@2 @0) (convert?@3 @1))) is extended.
It allows narrowing of arithmetic operation which has constant integer as its
second operand. It also simplifies next patch handling cond_expr.
Bootstrap and test on x86_64 and AArch64 for whole patch set. Is it OK?
Thanks,
bin
2016-10-21 Bin Cheng
* match.pd ((convert (op:s (convert@2 @0) (convert?@3 @1: Add
support for constant operand for OP.
gcc/testsuite/ChangeLog
2016-10-21 Bin Cheng
* gcc.dg/fold-narrowbopcst-1.c: New test.diff --git a/gcc/match.pd b/gcc/match.pd
index a8a6b85..86fb544 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -3335,7 +3335,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
operation and convert the result to the desired type. */
(for op (plus minus)
(simplify
-(convert (op:s (convert@2 @0) (convert@3 @1)))
+(convert (op:s (convert@2 @0) (convert?@3 @1)))
(if (INTEGRAL_TYPE_P (type)
/* We check for type compatibility between @0 and @1 below,
so there's no need to check that @1/@3 are integral types. */
@@ -3351,12 +3351,17 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
&& TYPE_PRECISION (type) == GET_MODE_PRECISION (TYPE_MODE (type))
/* The inner conversion must be a widening conversion. */
&& TYPE_PRECISION (TREE_TYPE (@2)) > TYPE_PRECISION (TREE_TYPE (@0))
-&& types_match (@0, @1)
-&& types_match (@0, type))
+&& types_match (@0, type)
+&& (types_match (@0, @1)
+/* Or the second operand must be constant integer. */
+|| (@3 == @1
+&& types_match (@1, @2)
+&& TREE_CODE (@1) == INTEGER_CST)))
(if (TYPE_OVERFLOW_WRAPS (TREE_TYPE (@0)))
- (convert (op @0 @1))
+ (convert (op @0 (convert:type @1)))
(with { tree utype = unsigned_type_for (TREE_TYPE (@0)); }
-(convert (op (convert:utype @0) (convert:utype @1
+(convert (op (convert:utype @0)
+ (convert:utype (convert:type @1)
/* This is another case of narrowing, specifically when there's an outer
BIT_AND_EXPR which masks off bits outside the type of the innermost
diff --git a/gcc/testsuite/gcc.dg/fold-narrowbopcst-1.c
b/gcc/testsuite/gcc.dg/fold-narrowbopcst-1.c
new file mode 100644
index 000..8a33677
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/fold-narrowbopcst-1.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-O1 -fdump-tree-optimized" } */
+
+int foo1 (unsigned char a[], unsigned int x)
+{
+ unsigned int i;
+ for (i = 0; i < 1000; i++)
+{
+ x = a[i];
+ a[i] = (unsigned char)(x >= 100 ? x - 100 : 0);
+}
+ return x;
+}
+/* { dg-final { scan-tree-dump " = _.* \\+ 156" "optimized" } } */
[PATCH GCC][4/4]Simplify (cond (cmp x c1) (op x c2) c3) -> (op (minmax x c1) c2)
Hi,
As commented in patch, this one simplifies (cond (cmp x c1) (op x c2) c3) into
(op (minmax x c1) c2) if:
1) OP is PLUS or MINUS.
2) CMP is LT, LE, GT or GE.
3) C3 == (C1 op C2), and the experation isn't undefined behavior.
This pattern also handles special cases like:
1) Comparison's operand x is a unsigned to signed type conversion
and c1 is integer zero. In this case,
(signed type)x < 0 <=> x > MAX_VAL(signed type)
(signed type)x >= 0 <=> x <= MAX_VAL(signed type)
2) Const c1 may not equal to (C3 op' C2). In this case we also
check equality for (c1+1) and (c1-1) by adjusting comparison
code.
Also note: Though signed type is handled by this pattern, it cannot be
simplified at the moment because C standard requires additional type promotion.
In order to match&simplify signed type cases, the IR needs to be cleaned up by
other optimizers, i.e, VRP.
For given loop:
int foo1 (unsigned short a[], unsigned int x)
{
unsigned int i;
for (i = 0; i < 1000; i++)
{
x = a[i];
a[i] = (unsigned short)(x <= 32768 ? x + 32768 : 0);
}
return x;
}
Generated assembly can be improved from:
.L4:
ldr q5, [x3, x1]
add w2, w2, 1
cmp w0, w2
ushll v1.4s, v5.4h, 0
ushll2 v0.4s, v5.8h, 0
add v4.4s, v1.4s, v2.4s
add v3.4s, v0.4s, v2.4s
cmhsv1.4s, v2.4s, v1.4s
cmhsv0.4s, v2.4s, v0.4s
and v1.16b, v4.16b, v1.16b
and v0.16b, v3.16b, v0.16b
xtn v3.4h, v1.4s
xtn2v3.8h, v0.4s
str q3, [x3, x1]
add x1, x1, 16
bhi .L4
To:
.L4:
ldr q1, [x3, x1]
add w2, w2, 1
cmp w0, w2
uminv0.8h, v1.8h, v2.8h
add v0.8h, v0.8h, v2.8h
str q0, [x3, x1]
add x1, x1, 16
bhi .L4
Bootstrap and test on x86_64 and AArch64 for whole patch set. Any comments?
Thanks,
bin
2016-10-21 Bin Cheng
* match.pd ((cond (cmp x c1) (op x c2) c3) -> (op (minmax x c1) c2)):
New pattern.
gcc/testsuite/ChangeLog
2016-10-21 Bin Cheng
* gcc.dg/fold-bopcond-1.c: New test.
* gcc.dg/fold-bopcond-2.c: New test.diff --git a/gcc/match.pd b/gcc/match.pd
index 071db2c..704421d 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -1981,6 +1981,109 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(max @1 @2)
(if (code == MIN_EXPR)
(min @1 @2))
+
+/* (cond (cmp x c1) (op x c2) c3) -> (op (minmax x c1) c2) if:
+
+ 1) OP is PLUS or MINUS.
+ 2) CMP is LT, LE, GT or GE.
+ 3) C3 == (C1 op C2), and the experation isn't undefined behavior.
+
+ This pattern also handles special cases like:
+
+ 1) Comparison's operand x is a unsigned to signed type conversion
+ and c1 is integer zero. In this case,
+ (signed type)x < 0 <=> x > MAX_VAL(signed type)
+ (signed type)x >= 0 <=> x <= MAX_VAL(signed type)
+ 2) Const c1 may not equal to (C3 op' C2). In this case we also
+ check equality for (c1+1) and (c1-1) by adjusting comparison
+ code.
+
+ TODO: Though signed type is handled by this pattern, it cannot be
+ simplified at the moment because C standard requires additional
+ promoting conversion. In order to match&simplify it here, the IR
+ needs to be cleaned up by other optimizers, i.e, VRP. */
+(for op (plus minus)
+ (for cmp (lt le gt ge)
+ (simplify
+ (cond (cmp@0 (convert?@00 @10) INTEGER_CST@01)
+(op @10 INTEGER_CST@11)
+INTEGER_CST@2)
+(with { tree from_type = TREE_TYPE (@10), to_type = TREE_TYPE (@00); }
+ (if (@00 == @10
+ || (TYPE_UNSIGNED (from_type)
+ && !TYPE_UNSIGNED (to_type)
+ && TYPE_PRECISION (from_type) == TYPE_PRECISION (to_type)
+ && integer_zerop (@01)
+ && (TREE_CODE (@0) == LT_EXPR || TREE_CODE (@0) == GE_EXPR)))
+ (with
+ {
+ bool overflow = false;
+ enum tree_code code, cmp_code = TREE_CODE (@0);
+ tree real_c1, c1 = @01, c2 = @11, c3 = @2;
+ tree op_type = TREE_TYPE (@10);
+ signop sgn = TYPE_SIGN (op_type);
+ widest_int wmin = widest_int::from (wi::min_value (op_type), sgn);
+ widest_int wmax = widest_int::from (wi::max_value (op_type), sgn);
+
+ /* Handle special cases, given x of unsigned type:
+ ((signed type)x < 0) <=> (x > MAX_VAL(signed type))
+ ((signed type)x >= 0) <=> (x <= MAX_VAL(signed type)) */
+ if (@00 != @10)
+ {
+ if (cmp_code == LT_EXPR)
+ cmp_code = GT_EXPR;
+ if (cmp_code == GE_EXPR)
+ cmp_code = LE_EXPR;
+ c1 = wide_int_to_tree (op_type, wi::max_value (to_type));
+ }
+ /* To simplify this pattern, we require c3 = (c1 op c2). Here we
+ compute (c3 op' c2) and check it equals to c1 in which op' is
+
[PATCH GCC][2/4]Simplify (cond (cmp (convert (x), c1), x, c2)) into (minmax (x, c))
Hi,
Second patch optimizing (cond (cmp (convert (x), c1), x, c2)) into (minmax (x,
c)). As commented in patch, this is done if:
+ 1) Comparison's operands are promoted from smaller type.
+ 2) Const c1 equals to c2 after canonicalizing comparison.
+ 3) Comparison has tree code LT, LE, GT or GE.
+This specific pattern is needed when (cmp (convert x) c) may not
+be simplified by comparison patterns because of multiple uses of
+x. It also makes sense here because simplifying across multiple
+referred var is always benefitial for complicated cases.
It also adds call to fold_stmt in tree-if-conv.c so that generated cond_expr
statement has its chance to be simplified.
Bootstrap and test on x86_64 and AArch64. It introduces below failure on both
x86_64 and AArch64:
FAIL: gcc.dg/vect/slp-cond-3.c
I believe it reveals defect in vect-slp. In call to fold_stmt in ifcvt,
canonicalization transforms _145 = _95 <= _96 ? _149 : _147 into _145 = _95 >
_96 ? _147 : _149. As a result, this stmt has different code to the first one
of SLP instance. IMO, SLP should be improved to handle operands swapping,
apparently, current support is not OK.
It also introduces more failures on AArch64(probably other targets) as below:
FAIL: gcc.dg/vect/pr65947-1.c -flto -ffat-lto-objects scan-tree-dump-times
vect "LOOP VECTORIZED" 2
FAIL: gcc.dg/vect/pr65947-1.c -flto -ffat-lto-objects scan-tree-dump-times
vect "condition expression based on integer induction." 4
FAIL: gcc.dg/vect/pr65947-1.c scan-tree-dump-times vect "LOOP VECTORIZED" 2
FAIL: gcc.dg/vect/pr65947-1.c scan-tree-dump-times vect "condition expression
based on integer induction." 4
FAIL: gcc.dg/vect/pr65947-13.c -flto -ffat-lto-objects scan-tree-dump-times
vect "LOOP VECTORIZED" 2
FAIL: gcc.dg/vect/pr65947-13.c scan-tree-dump-times vect "LOOP VECTORIZED" 2
FAIL: gcc.dg/vect/pr65947-4.c -flto -ffat-lto-objects scan-tree-dump-times
vect "LOOP VECTORIZED" 2
FAIL: gcc.dg/vect/pr65947-4.c -flto -ffat-lto-objects scan-tree-dump-times
vect "condition expression based on integer induction." 4
FAIL: gcc.dg/vect/pr65947-4.c scan-tree-dump-times vect "LOOP VECTORIZED" 2
FAIL: gcc.dg/vect/pr65947-4.c scan-tree-dump-times vect "condition expression
based on integer induction." 4
FAIL: gcc.dg/vect/pr77503.c -flto -ffat-lto-objects scan-tree-dump vect
"vectorized 1 loops"
FAIL: gcc.dg/vect/pr77503.c scan-tree-dump vect "vectorized 1 loops"
FAIL: gcc.dg/vect/vect-pr69848.c -flto -ffat-lto-objects scan-tree-dump vect
"vectorized 1 loops"
FAIL: gcc.dg/vect/vect-pr69848.c scan-tree-dump vect "vectorized 1 loops"
Again, these failures reveal a defect in vectorizer that operand swapping is
not supported for COND_REDUCTION.
I will send another two patches independent to this patch set resolving these
failures. Is this OK?
Thanks,
bin
2016-10-21 Bin Cheng
* tree-if-conv.c (ifcvt_follow_ssa_use_edges): New func.
(predicate_scalar_phi): Call fold_stmt using the new valueize func.
* match.pd ((cond (cmp (convert (x), c1), x, c2)) -> (minmax (x, c))):
New pattern.
gcc/testsuite/ChangeLog
2016-10-21 Bin Cheng
* gcc.dg/fold-condcmpconv-1.c: New test.
* gcc.dg/fold-condcmpconv-2.c: New test.diff --git a/gcc/match.pd b/gcc/match.pd
index 7365bc1..7523b2f 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -1930,6 +1930,59 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(if (integer_zerop (@0))
@2)))
+#if GIMPLE
+/* (cond (cmp (convert (x) c1) x c2)) -> (minmax (x c)) if:
+ 1) Comparison's operands are promoted from smaller type.
+ 2) Const c1 equals to c2 after canonicalizing comparison.
+ 3) Comparison has tree code LT, LE, GT or GE.
+This specific pattern is needed when (cmp (convert x) c) may not
+be simplified by comparison patterns because of multiple uses of
+x. It also makes sense here because simplifying across multiple
+referred var is always benefitial for complicated cases. */
+(for cmp (lt le gt ge)
+ (simplify
+ (cond (cmp@0 (convert@3 @1) INTEGER_CST@4) @1 INTEGER_CST@2)
+ (with
+{
+ tree op_type = TREE_TYPE (@1), cmp_type = TREE_TYPE (@3);
+ enum tree_code code = TREE_CODE (@0), cmp_code = TREE_CODE (@0);
+
+ if (TYPE_SIGN (cmp_type) == TYPE_SIGN (op_type)
+ && TYPE_PRECISION (cmp_type) > TYPE_PRECISION (op_type))
+ {
+ if (wi::to_widest (@4) == (wi::to_widest (@2) - 1))
+ {
+ /* X <= Y - 1 equals to X < Y. */
+ if (cmp_code == LE_EXPR)
+ code = LT_EXPR;
+ /* X > Y - 1 equals to X >= Y. */
+ if (cmp_code == GT_EXPR)
+ code = GE_EXPR;
+ }
+ if (wi::to_widest (@4) == (wi::to_widest (@2) + 1))
+ {
+ /* X < Y + 1 equals to X <= Y. */
+ if (cmp_code == LT_EXPR)
+ code = LE_EXPR;
+ /* X >= Y + 1 equals to X > Y. */
+ if (cmp_code ==
GIMPLE FE GSoC project now on gcc.gnu.org/git
I've moved (well, basically copied from the last sent patch) the GIMPLE FE GSoC project to a public branch on gcc.gnu.org git at remotes/origin/gimplefe (hopefully didn't mess stuff up too much). I've addressed Josephs stylistic comments already, work left is to move the GIMPLE parsing to a separate file as requested. I'm now going over the (few) middle-end changes to see whether they are good enough to be merged and I hope we can have a mostly working GIMPLE FE for GCC 7 for unit testing of GIMPLE passes. I'll happily review patches for the branch, please make sure to CC me if you send some. Thanks, Richard.
[PATCH] Fix 2 typos in IPA ICF pass
Hi.
Simple fix for a typo in IPA ICF. It cant' cause a regression as
cl_target_option_eq
function is called in equals_wpa function.
Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
Ready to be installed?
Martin
>From e4f9f913c32109a2773145870333ae88fc9cdae2 Mon Sep 17 00:00:00 2001
From: marxin
Date: Tue, 25 Oct 2016 09:57:55 +0200
Subject: [PATCH] Fix 2 typos in IPA ICF pass
gcc/testsuite/ChangeLog:
2016-10-25 Martin Liska
* gcc.dg/ipa/ipa-icf-32.c: Removed one scanned pattern.
gcc/ChangeLog:
2016-10-25 H.J. Lu
Martin Liska
PR ipa/78099
* common.opt: Mark flag_ipa_icf_variables as Optimization flag.
* ipa-icf.c (sem_function::get_hash): Add target optimization
node to hash.
---
gcc/common.opt| 2 +-
gcc/ipa-icf.c | 1 +
gcc/testsuite/gcc.dg/ipa/ipa-icf-32.c | 1 -
3 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/gcc/common.opt b/gcc/common.opt
index 6f24f56..729cbd8 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1597,7 +1597,7 @@ Common Report Var(flag_ipa_icf_functions) Optimization
Perform Identical Code Folding for functions.
fipa-icf-variables
-Common Report Var(flag_ipa_icf_variables)
+Common Report Var(flag_ipa_icf_variables) Optimization
Perform Identical Code Folding for variables.
fipa-reference
diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
index 3886fa6..e8880cb 100644
--- a/gcc/ipa-icf.c
+++ b/gcc/ipa-icf.c
@@ -300,6 +300,7 @@ sem_function::get_hash (void)
(cl_target_option_hash
(TREE_TARGET_OPTION (DECL_FUNCTION_SPECIFIC_TARGET (decl;
if (DECL_FUNCTION_SPECIFIC_OPTIMIZATION (decl))
+ hstate.add_wide_int
(cl_optimization_hash
(TREE_OPTIMIZATION (DECL_FUNCTION_SPECIFIC_OPTIMIZATION (decl;
hstate.add_flag (DECL_CXX_CONSTRUCTOR_P (decl));
diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-icf-32.c b/gcc/testsuite/gcc.dg/ipa/ipa-icf-32.c
index 9f42918..df11ac5 100644
--- a/gcc/testsuite/gcc.dg/ipa/ipa-icf-32.c
+++ b/gcc/testsuite/gcc.dg/ipa/ipa-icf-32.c
@@ -19,5 +19,4 @@ int main()
return foo (0) + bar (0);
}
-/* { dg-final { scan-ipa-dump "optimization flags are different" "icf" } } */
/* { dg-final { scan-ipa-dump "Equal symbols: 0" "icf" } } */
--
2.10.1
[PATCH] Fix not caught use-after-scope with -O1 (PR sanitize/78106)
Hello.
While reading and trying to understand sanopt code, I've noticed that we can
possibly
optimize out a ASAN_CHECK when there's a BB in between DOM(BB) and BB which can
call a freeing function.
Ready to be installed after it survives regression tests?
Martin
>From d8ed43c1f8e29cfe63ebd7c40a76715c9c644522 Mon Sep 17 00:00:00 2001
From: marxin
Date: Tue, 25 Oct 2016 13:29:47 +0200
Subject: [PATCH] Fix not caught use-after-scope with -O1 (PR sanitize/78106)
gcc/ChangeLog:
2016-10-25 Martin Liska
PR sanitizer/78106
* sanopt.c (imm_dom_path_with_freeing_call): Handle gasm
statements as they can also contain possibly a freeing call.
gcc/testsuite/ChangeLog:
2016-10-25 Martin Liska
PR sanitizer/78106
* gcc.dg/asan/pr78106.c: New test.
---
gcc/sanopt.c| 6 +-
gcc/testsuite/gcc.dg/asan/pr78106.c | 34 ++
2 files changed, 39 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.dg/asan/pr78106.c
diff --git a/gcc/sanopt.c b/gcc/sanopt.c
index 27c43da..8a6fbe9 100644
--- a/gcc/sanopt.c
+++ b/gcc/sanopt.c
@@ -211,8 +211,12 @@ imm_dom_path_with_freeing_call (basic_block bb, basic_block dom)
for (gsi = gsi_start_bb (e->src); !gsi_end_p (gsi); gsi_next (&gsi))
{
gimple *stmt = gsi_stmt (gsi);
+ gasm *asm_stmt;
- if (is_gimple_call (stmt) && !nonfreeing_call_p (stmt))
+ if ((is_gimple_call (stmt) && !nonfreeing_call_p (stmt))
+ || ((asm_stmt = dyn_cast (stmt))
+ && (gimple_asm_clobbers_memory_p (asm_stmt)
+ || gimple_asm_volatile_p (asm_stmt
{
pred_info->has_freeing_call_p = true;
break;
diff --git a/gcc/testsuite/gcc.dg/asan/pr78106.c b/gcc/testsuite/gcc.dg/asan/pr78106.c
new file mode 100644
index 000..7c0e05e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/asan/pr78106.c
@@ -0,0 +1,34 @@
+/* PR sanitizer/78106 */
+/* { dg-do run } */
+/* { dg-options "-fsanitize=address" } */
+/* { dg-shouldfail "asan" } */
+
+int *variable;
+
+void __attribute__((used)) release()
+{
+ __builtin_free (variable);
+}
+
+int main2(int argc)
+{
+ *variable = 2;
+
+ if (argc <= 5)
+asm volatile ("call release");
+
+ *variable = 2;
+ __builtin_abort ();
+
+ return 0;
+}
+
+int main(int argc, char **argv)
+{
+ variable = __builtin_malloc (sizeof(int));
+ return main2(argc);
+}
+
+/* { dg-output "ERROR: AddressSanitizer:? heap-use-after-free on address.*(\n|\r\n|\r)" } */
+/* { dg-output "WRITE of size 4 at.*" } */
+/* { dg-output "#0 0x\[0-9a-f\]+ +in _*main2 .*" } */
--
2.10.1
Re: [PATCH GCC][1/4]Simplify (convert1 (minmax ((convert2 (x) c)))) into minmax (x c)
On Tue, Oct 25, 2016 at 1:21 PM, Bin Cheng wrote:
> Hi,
> This is a patch set adding various match.pd patterns in order to generate
> simplified MIN/MAX_EXPR mostly from COND_EXPR. This is the first one
> optimizing (convert1 (minmax ((convert2 (x) c to minmax (x c), if
> convert2 promotes x and convert1 demotes back to x's type. With this patch,
> generated assembly for test:
> .L4:
> ldr q0, [x3, x1]
> add w2, w2, 1
> cmp w0, w2
> ushll v2.4s, v0.4h, 0
> ushll2 v1.4s, v0.8h, 0
> uminv2.4s, v2.4s, v3.4s
> uminv1.4s, v1.4s, v3.4s
> xtn v4.4h, v2.4s
> xtn2v4.8h, v1.4s
> str q4, [x3, x1]
> add x1, x1, 16
> bhi .L4
>
> can be improved to:
> .L4:
> ldr q0, [x3, x1]
> add w2, w2, 1
> cmp w0, w2
> uminv1.8h, v0.8h, v2.8h
> str q1, [x3, x1]
> add x1, x1, 16
> bhi .L4
>
> Bootstrap and test on x86_64 and AArch64 for whole patch set. Is it OK?
Why restrict to GIMPLE?
+/* (convert1 (minmax ((convert2 (x) c -> minmax (x c) if convert2
+ promotes x and convert1 demotes back to x's type. */
+
+(for minmax (min max)
+ (simplify
+ (convert (minmax@0 (convert @1) INTEGER_CST@2))
+ (if (types_compatible_p (TREE_TYPE (@1), type))
comment mentions convert1 and convert2, just convert is correct I think.
Please use types_match instead of types_compatible_p, this is a
wrapper that does the correct thing for both GENERIC and GIMPLE.
+ (with
+{
+ tree minmax_type = TREE_TYPE (@0);
+ signop sgn = TYPE_SIGN (type);
+ widest_int type_min = widest_int::from (wi::min_value (type), sgn);
+ widest_int type_max = widest_int::from (wi::max_value (type), sgn);
+}
+(if (sgn == TYPE_SIGN (minmax_type)
+&& TYPE_PRECISION (minmax_type) >= TYPE_PRECISION (type)
+&& wi::to_widest (@2) >= type_min && wi::to_widest (@2) <= type_max)
instead of this you can use int_fits_type_p (type, @2)
+ (minmax @1 { fold_convert (type, @2); }))
use
(minmax @1 (convert @2))
I believe the transform is also a win if @2 is not a constant but similarly
promoted as @1. This slightly complicates the patter and thus can
be done as followup (if we think it's useful at this point).
With the simplification you should get rid of the with{}
Thanks,
Richard.
> Thanks,
> bin
>
> 2016-10-21 Bin Cheng
>
> * match.pd ((convert1 (minmax ((convert2 (x) c -> minmax (x c)):
> New pattern.
>
> gcc/testsuite/ChangeLog
> 2016-10-21 Bin Cheng
>
> * gcc.dg/fold-convmaxconv-1.c: New test.
> * gcc.dg/fold-convminconv-1.c: New test.
Re: [PATCH] Fix not caught use-after-scope with -O1 (PR sanitize/78106)
On Tue, Oct 25, 2016 at 01:40:03PM +0200, Martin Liška wrote: > While reading and trying to understand sanopt code, I've noticed that we can > possibly > optimize out a ASAN_CHECK when there's a BB in between DOM(BB) and BB which > can > call a freeing function. > > Ready to be installed after it survives regression tests? > Martin > >From d8ed43c1f8e29cfe63ebd7c40a76715c9c644522 Mon Sep 17 00:00:00 2001 > From: marxin > Date: Tue, 25 Oct 2016 13:29:47 +0200 > Subject: [PATCH] Fix not caught use-after-scope with -O1 (PR sanitize/78106) > > gcc/ChangeLog: > > 2016-10-25 Martin Liska > > PR sanitizer/78106 > * sanopt.c (imm_dom_path_with_freeing_call): Handle gasm > statements as they can also contain possibly a freeing call. Other places use something like if ((gimple_code (stmt) == GIMPLE_ASM && gimple_vdef (stmt)) || (is_gimple_call (stmt) && (!nonfreeing_call_p (stmt) || !nonbarrier_call_p (stmt though what you added matches more what ipa-pure-const.c does, ok. > gcc/testsuite/ChangeLog: > > 2016-10-25 Martin Liska > > PR sanitizer/78106 > * gcc.dg/asan/pr78106.c: New test. The test is bad. 1) asan is supported on various architectures, call release is x86 specific, and even there on some OSes the syntax might be different (_release, etc.?) 2) you aren't trying to maintain required stack alignment So, I think it would be better to just use dg-do compile and just scan some dump. Jakub
Re: [PATCH GCC][3/4]Add support for constant operand in pattern (convert (op:s (convert@2 @0) (convert?@3 @1)))
On Tue, Oct 25, 2016 at 1:21 PM, Bin Cheng wrote:
> Hi,
> This is an update patch for
> https://gcc.gnu.org/ml/gcc-patches/2016-10/msg00738.html . In this version,
> existing pattern (convert (op:s (convert@2 @0) (convert?@3 @1))) is extended.
> It allows narrowing of arithmetic operation which has constant integer as
> its second operand. It also simplifies next patch handling cond_expr.
> Bootstrap and test on x86_64 and AArch64 for whole patch set. Is it OK?
+&& types_match (@0, type)
+&& (types_match (@0, @1)
+/* Or the second operand must be constant integer. */
+|| (@3 == @1
+&& types_match (@1, @2)
+&& TREE_CODE (@1) == INTEGER_CST)))
So this fails to match the pattern if we get into it via valueization
and get, say,
(short)((int)a + (int)7). I believe for plus and minus we're always safe so
I suggest to simply do
&& (types_match (@0, @1)
|| TREE_CODE (@1) == INTEGER_CST)
(if (TYPE_OVERFLOW_WRAPS (TREE_TYPE (@0)))
- (convert (op @0 @1))
+ (convert (op @0 (convert:type @1)))
:type shouldn't be necessary -- it also shows the outer (convert ..)
is not required,
please remove it while you're here.
(with { tree utype = unsigned_type_for (TREE_TYPE (@0)); }
-(convert (op (convert:utype @0) (convert:utype @1
+(convert (op (convert:utype @0)
+ (convert:utype (convert:type @1)
Why do you need the intermediate conversion?
Richard.
> Thanks,
> bin
>
> 2016-10-21 Bin Cheng
>
> * match.pd ((convert (op:s (convert@2 @0) (convert?@3 @1: Add
> support for constant operand for OP.
>
> gcc/testsuite/ChangeLog
> 2016-10-21 Bin Cheng
>
> * gcc.dg/fold-narrowbopcst-1.c: New test.
Re: [PATCH] Fix 2 typos in IPA ICF pass
On Tue, Oct 25, 2016 at 1:37 PM, Martin Liška wrote: > Hi. > > Simple fix for a typo in IPA ICF. It cant' cause a regression as > cl_target_option_eq > function is called in equals_wpa function. > > Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. > > Ready to be installed? Ok. Richard. > Martin
Re: RFC [1/3] divmod transform v2
On 25 October 2016 at 16:17, Richard Biener wrote:
> On Tue, 25 Oct 2016, Prathamesh Kulkarni wrote:
>
>> On 25 October 2016 at 13:43, Richard Biener
>> wrote:
>> > On Sun, Oct 16, 2016 at 7:59 AM, Prathamesh Kulkarni
>> > wrote:
>> >> Hi,
>> >> After approval from Bernd Schmidt, I committed the patch to remove
>> >> optab functions for
>> >> sdivmod_optab and udivmod_optab in optabs.def, which removes the block
>> >> for divmod patch.
>> >>
>> >> This patch is mostly the same as previous one, except it drops
>> >> targeting __udivmoddi4() because
>> >> it gave undefined reference link error for calling __udivmoddi4() on
>> >> aarch64-linux-gnu.
>> >> It appears aarch64 has hardware insn for DImode div, so __udivmoddi4()
>> >> isn't needed for the target
>> >> (it was a bug in my patch that called __udivmoddi4() even though
>> >> aarch64 supported hardware div).
>> >>
>> >> However this makes me wonder if it's guaranteed that __udivmoddi4()
>> >> will be available for a target if it doesn't have hardware div and
>> >> divmod insn and doesn't have target-specific libfunc for
>> >> DImode divmod ? To be conservative, the attached patch doesn't
>> >> generate call to __udivmoddi4.
>> >>
>> >> Passes bootstrap+test on x86_64-unknown-linux.
>> >> Cross-tested on arm*-*-*, aarch64*-*-*.
>> >> Verified that there are no regressions with SPEC2006 on
>> >> x86_64-unknown-linux-gnu.
>> >> OK to commit ?
>> >
>> > I think the searching is still somewhat wrong - it's been some time
>> > since my last look at the
>> > patch so maybe I've said this already. Please bail out early for
>> > stmt_can_throw_internal (stmt),
>> > otherwise the top stmt search might end up not working. So
>> >
>> > +
>> > + if (top_stmt == stmt && stmt_can_throw_internal (top_stmt))
>> > +return false;
>> >
>> > can go.
>> >
>> > top_stmt may end up as a TRUNC_DIV_EXPR so it's pointless to only look
>> > for another
>> > TRUNC_DIV_EXPR later ... you may end up without a single TRUNC_MOD_EXPR.
>> > Which means you want a div_seen and a mod_seen, or simply record the
>> > top_stmt
>> > code and look for the opposite in the 2nd loop.
>> Um sorry I don't quite understand how we could end up without a trunc_mod
>> stmt ?
>> The 2nd loop adds both trunc_div and trunc_mod to stmts vector, and
>> checks if we have
>> come across at least a single trunc_div stmt (and we bail out if no
>> div is seen).
>>
>> At 2nd loop I suppose we don't need mod_seen, because stmt is
>> guaranteed to be trunc_mod_expr.
>> In the 2nd loop the following condition will never trigger for stmt:
>> if (stmt_can_throw_internal (use_stmt))
>> continue;
>> since we checked before hand if stmt could throw and chose to bail out
>> in that case.
>>
>> and the following condition would also not trigger for stmt:
>> if (!dominated_by_p (CDI_DOMINATORS, gimple_bb (use_stmt), top_bb))
>> {
>> end_imm_use_stmt_traverse (&use_iter);
>> return false;
>> }
>> since gimple_bb (stmt) is always dominated by gimple_bb (top_stmt).
>>
>> The case where top_stmt == stmt, we wouldn't reach the above
>> condition, since we have above it:
>> if (top_stmt == stmt)
>> continue;
>>
>> So IIUC, top_stmt and stmt would always get added to stmts vector.
>> Am I missing something ?
>
> Ah, indeed. Maybe add a comment then, it wasn't really obvious ;)
>
> Please still move the stmt_can_throw_internal (stmt) check up.
Sure, I will move that up and do the other suggested changes.
I was wondering if this condition in 2nd loop is too restrictive ?
if (!dominated_by_p (CDI_DOMINATORS, gimple_bb (use_stmt), top_bb))
{
end_imm_use_stmt_traverse (&use_iter);
return false;
}
Should we rather "continue" in this case by not adding use_stmt to
stmts vector rather than dropping
the transform all-together if gimple_bb (use_stmt) is not dominated by
gimple_bb (top_stmt) ?
For instance if we have a test-case like:
if (cond)
{
t1 = x / y;
t2 = x % y;
}
else
t3 = x % y;
and suppose stmt is "t2 = x % y", we would set top_stmt to "t1 = x / y";
In this case we would still want to do divmod transform in THEN block
even though "t3 = x % y" is not dominated by top_stmt ?
if (cond)
{
divmod_tmp = DIVMOD (x, y);
t1 = REALPART_EXPR (divmod_tmp);
t2 = IMAGPART_EXPR (divmod_tmp);
}
else
t3 = x % y;
We will always ensure that all the trunc_div, trunc_mod statements in
stmts vector will be dominated by top_stmt,
but I suppose they need not constitute all the trunc_div, trunc_mod
statements in the function.
Thanks,
Prathamesh
>
> Thanks,
> Richard.
>
>> Thanks,
>> Prathamesh
>> >
>> > + switch (gimple_assign_rhs_code (use_stmt))
>> > + {
>> > + case TRUNC_DIV_EXPR:
>> > + new_rhs = fold_build1 (REALPART_EXPR, TREE_TYPE (op1), res);
>> > + break;
>> > +
>> > + case TRUNC_MOD_EXPR:
>> > + new_rhs = fold_build1 (IMAGPART_EXPR, TREE_TYPE (op2), res);
>> > + break;
>> > +
>> >
>> > why type of op1 and
Re: [PATCH] Fix not caught use-after-scope with -O1 (PR sanitize/78106)
On 10/25/2016 01:52 PM, Jakub Jelinek wrote:
> On Tue, Oct 25, 2016 at 01:40:03PM +0200, Martin Liška wrote:
>> While reading and trying to understand sanopt code, I've noticed that we can
>> possibly
>> optimize out a ASAN_CHECK when there's a BB in between DOM(BB) and BB which
>> can
>> call a freeing function.
>>
>> Ready to be installed after it survives regression tests?
>> Martin
>
>> >From d8ed43c1f8e29cfe63ebd7c40a76715c9c644522 Mon Sep 17 00:00:00 2001
>> From: marxin
>> Date: Tue, 25 Oct 2016 13:29:47 +0200
>> Subject: [PATCH] Fix not caught use-after-scope with -O1 (PR sanitize/78106)
>>
>> gcc/ChangeLog:
>>
>> 2016-10-25 Martin Liska
>>
>> PR sanitizer/78106
>> * sanopt.c (imm_dom_path_with_freeing_call): Handle gasm
>> statements as they can also contain possibly a freeing call.
>
> Other places use something like
> if ((gimple_code (stmt) == GIMPLE_ASM && gimple_vdef (stmt))
> || (is_gimple_call (stmt)
> && (!nonfreeing_call_p (stmt) || !nonbarrier_call_p (stmt
> though what you added matches more what ipa-pure-const.c does, ok.
Yes, I basically took the same code as we already have on a different place in
the file:
sanopt.c:531
if (!is_gimple_call (stmt))
{
/* Handle asm volatile or asm with "memory" clobber
the same as potentionally freeing call. */
gasm *asm_stmt = dyn_cast (stmt);
if (asm_stmt
&& asan_check_optimize
&& (gimple_asm_clobbers_memory_p (asm_stmt)
|| gimple_asm_volatile_p (asm_stmt)))
info->freeing_call_events++;
gsi_next (&gsi);
continue;
}
if (asan_check_optimize && !nonfreeing_call_p (stmt))
info->freeing_call_events++;
>
>> gcc/testsuite/ChangeLog:
>>
>> 2016-10-25 Martin Liska
>>
>> PR sanitizer/78106
>> * gcc.dg/asan/pr78106.c: New test.
>
> The test is bad. 1) asan is supported on various architectures, call release
> is x86 specific, and even there on some OSes the syntax might be different
> (_release, etc.?) 2) you aren't trying to maintain required stack alignment
>
> So, I think it would be better to just use dg-do compile and just scan some
> dump.
I see! I changed the code to scan sanopts-details dump file.
Ready to install with the change?
Thanks,
Martin
>
> Jakub
>
>From aed3af4fb992a58c77d9f5b2ef9a70de5dff8aa8 Mon Sep 17 00:00:00 2001
From: marxin
Date: Tue, 25 Oct 2016 13:29:47 +0200
Subject: [PATCH] Fix not caught use-after-scope with -O1 (PR sanitize/78106)
gcc/ChangeLog:
2016-10-25 Martin Liska
PR sanitizer/78106
* sanopt.c (imm_dom_path_with_freeing_call): Handle gasm
statements as they can also contain possibly a freeing call.
gcc/testsuite/ChangeLog:
2016-10-25 Martin Liska
PR sanitizer/78106
* gcc.dg/asan/pr78106.c: New test.
---
gcc/sanopt.c| 6 +-
gcc/testsuite/gcc.dg/asan/pr78106.c | 31 +++
2 files changed, 36 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.dg/asan/pr78106.c
diff --git a/gcc/sanopt.c b/gcc/sanopt.c
index 27c43da..8a6fbe9 100644
--- a/gcc/sanopt.c
+++ b/gcc/sanopt.c
@@ -211,8 +211,12 @@ imm_dom_path_with_freeing_call (basic_block bb, basic_block dom)
for (gsi = gsi_start_bb (e->src); !gsi_end_p (gsi); gsi_next (&gsi))
{
gimple *stmt = gsi_stmt (gsi);
+ gasm *asm_stmt;
- if (is_gimple_call (stmt) && !nonfreeing_call_p (stmt))
+ if ((is_gimple_call (stmt) && !nonfreeing_call_p (stmt))
+ || ((asm_stmt = dyn_cast (stmt))
+ && (gimple_asm_clobbers_memory_p (asm_stmt)
+ || gimple_asm_volatile_p (asm_stmt
{
pred_info->has_freeing_call_p = true;
break;
diff --git a/gcc/testsuite/gcc.dg/asan/pr78106.c b/gcc/testsuite/gcc.dg/asan/pr78106.c
new file mode 100644
index 000..d333f9b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/asan/pr78106.c
@@ -0,0 +1,31 @@
+/* PR sanitizer/78106 */
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=address -fdump-tree-sanopt-details" } */
+
+int *variable;
+
+void __attribute__((used)) release()
+{
+ __builtin_free (variable);
+}
+
+int main2(int argc)
+{
+ *variable = 2;
+
+ if (argc <= 5)
+asm volatile ("call release");
+
+ *variable = 2;
+ __builtin_abort ();
+
+ return 0;
+}
+
+int main(int argc, char **argv)
+{
+ variable = __builtin_malloc (sizeof(int));
+ return main2(argc);
+}
+
+/* { dg-final { scan-tree-dump-not "Optimizing out(\n|\r\n|\r) ASAN_CHECK \\(7, variable.*" "sanopt" } } */
--
2.10.1
Re: Add uniform_inside_sphere_distribution
All,
Here is the library extension for uniform_inside _sphere_distribution.
It works from discs and has been tested up through 12-dimentional spheres.
The patch dispatches to rejection for Dim<8, transform otherwise as
discussed earlier.
Builds and tests cleanly on x86_64-linux.
OK?
Ed
Index: include/ext/random
===
--- include/ext/random (revision 241499)
+++ include/ext/random (working copy)
@@ -3493,6 +3493,218 @@
_RealType>& __d2)
{ return !(__d1 == __d2); }
+
+ /**
+ * @brief A distribution for random coordinates inside a unit sphere.
+ */
+ template
+class uniform_inside_sphere_distribution
+{
+ static_assert(std::is_floating_point<_RealType>::value,
+ "template argument not a floating point type");
+ static_assert(_Dimen != 0, "dimension is zero");
+
+public:
+ /** The type of the range of the distribution. */
+ using result_type = std::array<_RealType, _Dimen>;
+
+ /** Parameter type. */
+ struct param_type
+ {
+ using distribution_type
+ = uniform_inside_sphere_distribution<_Dimen, _RealType>;
+ friend class uniform_inside_sphere_distribution<_Dimen, _RealType>;
+
+ explicit
+ param_type(_RealType __radius = _RealType(1))
+ : _M_radius(__radius)
+ {
+ _GLIBCXX_DEBUG_ASSERT(_M_radius > _RealType(0));
+ }
+
+ _RealType
+ radius() const
+ { return _M_radius; }
+
+ friend bool
+ operator==(const param_type& __p1, const param_type& __p2)
+ { return __p1._M_radius == __p2._M_radius; }
+
+ private:
+ _RealType _M_radius;
+ };
+
+ /**
+ * @brief Constructors.
+ */
+ explicit
+ uniform_inside_sphere_distribution(_RealType __radius = _RealType(1))
+ : _M_param(__radius), _M_uosd()
+ { }
+
+ explicit
+ uniform_inside_sphere_distribution(const param_type& __p)
+ : _M_param(__p), _M_uosd()
+ { }
+
+ /**
+ * @brief Resets the distribution state.
+ */
+ void
+ reset()
+ { _M_uosd.reset(); }
+
+ /**
+ * @brief Returns the @f$radius@f$ of the distribution.
+ */
+ _RealType
+ radius() const
+ { return _M_param.radius(); }
+
+ /**
+ * @brief Returns the parameter set of the distribution.
+ */
+ param_type
+ param() const
+ { return _M_param; }
+
+ /**
+ * @brief Sets the parameter set of the distribution.
+ * @param __param The new parameter set of the distribution.
+ */
+ void
+ param(const param_type& __param)
+ { _M_param = __param; }
+
+ /**
+ * @brief Returns the greatest lower bound value of the distribution.
+ * This function makes no sense for this distribution.
+ */
+ result_type
+ min() const
+ {
+ result_type __res;
+ __res.fill(0);
+ return __res;
+ }
+
+ /**
+ * @brief Returns the least upper bound value of the distribution.
+ * This function makes no sense for this distribution.
+ */
+ result_type
+ max() const
+ {
+ result_type __res;
+ __res.fill(0);
+ return __res;
+ }
+
+ /**
+ * @brief Generating functions.
+ */
+ template
+ result_type
+ operator()(_UniformRandomNumberGenerator& __urng)
+ { return this->operator()(__urng, _M_param); }
+
+ template
+ result_type
+ operator()(_UniformRandomNumberGenerator& __urng,
+ const param_type& __p);
+
+ template
+ void
+ __generate(_ForwardIterator __f, _ForwardIterator __t,
+ _UniformRandomNumberGenerator& __urng)
+ { this->__generate(__f, __t, __urng, this->param()); }
+
+ template
+ void
+ __generate(_ForwardIterator __f, _ForwardIterator __t,
+ _UniformRandomNumberGenerator& __urng,
+ const param_type& __p)
+ { this->__generate_impl(__f, __t, __urng, __p); }
+
+ template
+ void
+ __generate(result_type* __f, result_type* __t,
+ _UniformRandomNumberGenerator& __urng,
+ const param_type& __p)
+ { this->__generate_impl(__f, __t, __urng, __p); }
+
+ /**
+ * @brief Return true if two uniform on sphere distributions have
+ *the same parameters and the sequences that would be
+ *generated are equal.
+ */
+ friend bool
+ operator==(const uniform_inside_sphere_distribution& __d1,
+const uniform_inside_sphere_distribution& __d2)
+ { return __d1._M_param == __d2._M_param && __d1._M_uosd == __d2._M_uosd;
}
+
+ /**
+ * @brief Inserts a %uniform_inside_sphere_distribution random number
+ *distribution @p __x into the output stream @p __os.
+ *
+ * @param
Re: [PATCH] print_rtx: implement support for reuse IDs
On 10/21/2016 10:27 PM, David Malcolm wrote:
Thanks. I attemped to use those fields of recog_data, but it doesn't
seem to be exactly what's needed here.
Yeah, I may have been confused. I'm not sure that just looking at
SCRATCHes is the right thing either, but I think you're on the right
track, and we can use something like your patch for now and extend it
later if necessary.
+ public:
+ rtx_reuse_manager ();
+ ~rtx_reuse_manager ();
+ static rtx_reuse_manager *get () { return singleton; }
OTOH, this setup looks a bit odd to me. Are you trying to avoid
converting the print_rtx stuff to its own class, or avoid passing the
reuse manager as an argument to a lot of functions?
Some of this setup might not even be necessary. We have a "used" flag on
rtx objects which is used to unshare RTL, and I think could also be used
for a similar purpose when dumping. So, before printing, call
reset_insn_used_flags on everything, then have another pass to set bits
on everything that could conceivably be shared, and when you find
something that already has the bit set, enter it into a table. Finally,
print everything out, using the table. I think this would be somewhat
simpler than adding another header file and class definition.
+void
+rtx_reuse_manager::preprocess (const_rtx x)
+{
+ subrtx_iterator::array_type array;
+ FOR_EACH_SUBRTX (iter, array, x, NONCONST)
+if (uses_rtx_reuse_p (*iter))
+ {
+ if (int *count = m_rtx_occurrence_count.get (*iter))
+ {
+ if (*count == 1)
+ {
+ m_rtx_reuse_ids.put (*iter, m_next_id++);
+ }
+ (*count)++;
+ }
+ else
+ m_rtx_occurrence_count.put (*iter, 1);
+ }
Formatting rules suggest no braces around single statements, I think a
more readable version of this would be:
if (uses_rtx_reuse_p (*iter))
{
int *count = m_rtx_occurrence_count.get (*iter)
if (count)
{
if ((*count)++ == 1)
m_rtx_reuse_ids.put (*iter, m_next_id++);
}
else
m_rtx_occurrence_count.put (*iter, 1);
}
Bernd
Re: RFC [1/3] divmod transform v2
On Tue, 25 Oct 2016, Prathamesh Kulkarni wrote:
> On 25 October 2016 at 16:17, Richard Biener wrote:
> > On Tue, 25 Oct 2016, Prathamesh Kulkarni wrote:
> >
> >> On 25 October 2016 at 13:43, Richard Biener
> >> wrote:
> >> > On Sun, Oct 16, 2016 at 7:59 AM, Prathamesh Kulkarni
> >> > wrote:
> >> >> Hi,
> >> >> After approval from Bernd Schmidt, I committed the patch to remove
> >> >> optab functions for
> >> >> sdivmod_optab and udivmod_optab in optabs.def, which removes the block
> >> >> for divmod patch.
> >> >>
> >> >> This patch is mostly the same as previous one, except it drops
> >> >> targeting __udivmoddi4() because
> >> >> it gave undefined reference link error for calling __udivmoddi4() on
> >> >> aarch64-linux-gnu.
> >> >> It appears aarch64 has hardware insn for DImode div, so __udivmoddi4()
> >> >> isn't needed for the target
> >> >> (it was a bug in my patch that called __udivmoddi4() even though
> >> >> aarch64 supported hardware div).
> >> >>
> >> >> However this makes me wonder if it's guaranteed that __udivmoddi4()
> >> >> will be available for a target if it doesn't have hardware div and
> >> >> divmod insn and doesn't have target-specific libfunc for
> >> >> DImode divmod ? To be conservative, the attached patch doesn't
> >> >> generate call to __udivmoddi4.
> >> >>
> >> >> Passes bootstrap+test on x86_64-unknown-linux.
> >> >> Cross-tested on arm*-*-*, aarch64*-*-*.
> >> >> Verified that there are no regressions with SPEC2006 on
> >> >> x86_64-unknown-linux-gnu.
> >> >> OK to commit ?
> >> >
> >> > I think the searching is still somewhat wrong - it's been some time
> >> > since my last look at the
> >> > patch so maybe I've said this already. Please bail out early for
> >> > stmt_can_throw_internal (stmt),
> >> > otherwise the top stmt search might end up not working. So
> >> >
> >> > +
> >> > + if (top_stmt == stmt && stmt_can_throw_internal (top_stmt))
> >> > +return false;
> >> >
> >> > can go.
> >> >
> >> > top_stmt may end up as a TRUNC_DIV_EXPR so it's pointless to only look
> >> > for another
> >> > TRUNC_DIV_EXPR later ... you may end up without a single TRUNC_MOD_EXPR.
> >> > Which means you want a div_seen and a mod_seen, or simply record the
> >> > top_stmt
> >> > code and look for the opposite in the 2nd loop.
> >> Um sorry I don't quite understand how we could end up without a trunc_mod
> >> stmt ?
> >> The 2nd loop adds both trunc_div and trunc_mod to stmts vector, and
> >> checks if we have
> >> come across at least a single trunc_div stmt (and we bail out if no
> >> div is seen).
> >>
> >> At 2nd loop I suppose we don't need mod_seen, because stmt is
> >> guaranteed to be trunc_mod_expr.
> >> In the 2nd loop the following condition will never trigger for stmt:
> >> if (stmt_can_throw_internal (use_stmt))
> >> continue;
> >> since we checked before hand if stmt could throw and chose to bail out
> >> in that case.
> >>
> >> and the following condition would also not trigger for stmt:
> >> if (!dominated_by_p (CDI_DOMINATORS, gimple_bb (use_stmt), top_bb))
> >> {
> >> end_imm_use_stmt_traverse (&use_iter);
> >> return false;
> >> }
> >> since gimple_bb (stmt) is always dominated by gimple_bb (top_stmt).
> >>
> >> The case where top_stmt == stmt, we wouldn't reach the above
> >> condition, since we have above it:
> >> if (top_stmt == stmt)
> >> continue;
> >>
> >> So IIUC, top_stmt and stmt would always get added to stmts vector.
> >> Am I missing something ?
> >
> > Ah, indeed. Maybe add a comment then, it wasn't really obvious ;)
> >
> > Please still move the stmt_can_throw_internal (stmt) check up.
> Sure, I will move that up and do the other suggested changes.
>
> I was wondering if this condition in 2nd loop is too restrictive ?
> if (!dominated_by_p (CDI_DOMINATORS, gimple_bb (use_stmt), top_bb))
> {
> end_imm_use_stmt_traverse (&use_iter);
> return false;
> }
>
> Should we rather "continue" in this case by not adding use_stmt to
> stmts vector rather than dropping
> the transform all-together if gimple_bb (use_stmt) is not dominated by
> gimple_bb (top_stmt) ?
Ah, yes - didn't spot that.
Richard.
>
> For instance if we have a test-case like:
>
> if (cond)
> {
> t1 = x / y;
> t2 = x % y;
> }
> else
> t3 = x % y;
>
> and suppose stmt is "t2 = x % y", we would set top_stmt to "t1 = x / y";
> In this case we would still want to do divmod transform in THEN block
> even though "t3 = x % y" is not dominated by top_stmt ?
>
> if (cond)
> {
> divmod_tmp = DIVMOD (x, y);
> t1 = REALPART_EXPR (divmod_tmp);
> t2 = IMAGPART_EXPR (divmod_tmp);
> }
> else
> t3 = x % y;
>
> We will always ensure that all the trunc_div, trunc_mod statements in
> stmts vector will be dominated by top_stmt,
> but I suppose they need not constitute all the trunc_div, trunc_mod
> statements in the function.
>
> Thanks,
> Prathamesh
> >
> > Thanks,
> > Richard.
> >
> >> Thanks,
> >> Prathamesh
> >> >
> >> > + switch (gimp
Re: [PATCH, RFC] Introduce -fsanitize=use-after-scope (v2)
On 10/21/2016 04:26 PM, Jakub Jelinek wrote: > My memory is weak, but isn't this something the sanopt pass > (sanopt_optimize) is already doing similarly for e.g. ASAN_CHECK, UBSAN_NULL > and UBSAN_VPTR checks? For ASAN_MARK, you actually don't care about any > calls, those shouldn't unpoison or poison again the vars under compiler's > back. Hi Jakub. Your memory is not weak, it's exactly what to pass does. I've spent quite some time reading and understanding the optimization (and finding PR78106) and looks very similar to what we have for ASAN_MARK. However it's more complicated due to existence of poisoning and unpoisoning calls. In the previous email, you summarized what "patterns" can be optimized out: 1) - optimization to remove ASAN_MARK unpoisoning at the start of the function 2) - optimization to remove ASAN_MARK poisoning at the end of function (and remove epilogue unpoisoning) 3) - optimization to remove ASAN_MARK unpoisoning followed by ASAN_MARK poisoning or vice versa if there are no memory accesses in between I'll come with more generalization: Ia) ASAN_MARK poisonining followed by ASAN_MARK unpoisoning with no memory access in between => ASAN_MARK poisoning can be removed; Ib) ASAN_MARK unpoisonining followed by ASAN_MARK poisoning with no memory access in between => ASAN_MARK unpoisoning can be removed; Ia + Ib match exactly with 3). As function epilogue contains unpoisoning, Ia) == 2) IIa) ASAN_MARK poisoning followed by ASAN_MARK poisoning => second ASAN_MARK can be removed IIb) ASAN_MARK unpoisoning followed by ASAN_MARK unpoisoning => second ASAN_MARK can be removed IIb matches 1) III) ASAN_CHECK(&var) followed by ASAN_CHECK(&var) with no ASAN_MARK poisoning in between => second ASAN_CHECK can be removed (this is more aggressive than what current can do maybe_optimize_asan_check_ifn). Notes: - Compared to current imm_dom_path_with_freeing_call_p, we would need 3 such predicates: x) imm_dom_path_with_asan_check () - needed by Ia, Ib y) imm_dom_path_with_asan_mark_poisoning () - this should return set of variables which can be potentially poisoned - needed by IIb and III z) imm_dom_path_with_asan_mark_unpoisoning () - this should return set of variables which can be potentially unpoisoned - needed by IIa I would appreciate a feedback about my brainstorming I did about sanopt porting to use-after-scope, maybe my cases are more generic that we would eventually need. And I'm not 100% convinced that all my patterns described above would be doable in a similar way as current sanopt algorithm is done? Thanks, Martin
[PATCH][GIMPLE FE] Split out parser into separate file
Hi,
so I did the massaging to split out the GIMPLE parsing routines out
to a separate file (quite tricky to get the gengtype issues correctly
so I thought to help out here and get things started).
Tested on x86_64-unknown-linux-gnu, bootstrap is still in progress.
Will push to gimplefe soon.
I think it shows that the GIMPLE FE should maybe not use
c_parser_cast_expression or c_token_starts_typename or
c_parser_next_token_starts_declspecs. But maybe it's not
100% avoidable as we want to re-use C parsing for local
declarations (of types and decls) as well (eventually even
local statics with initializers which remain GENERIC).
Richard.
commit f2dc45b7d1cbd3122944a26f449d7fd233994f50
Author: Richard Guenther
Date: Tue Oct 25 15:20:42 2016 +0200
2016-10-25 Richard Biener
c/
* Make-lang.in (C_AND_OBJC_OBJS): Add gimple-parser.o.
* config-lang.in (gtfiles): Add c/c-parser.h.
* c-lang.c: Include c-pragma.h and c-parser.h.
* c-parser.c (enum c_id_kind, struct c_token,
struct c_parser, c_parser_next_token_is,
c_parser_next_token_is_not,
c_parser_next_token_is_keyword,
enum c_lookahead_kind, enum c_dtr_syn, enum c_parser_prec):
Split out to ...
* c-parser.h: ... new header.
* c-parser.c (c_parser_peek_token, c_parser_peek_2nd_token,
c_token_starts_typename, c_parser_next_token_starts_declspecs,
c_parser_next_tokens_start_declaration, c_parser_consume_token,
c_parser_error, c_parser_require, c_parser_skip_until_found,
c_parser_declspecs, c_parser_declarator,
c_parser_cast_expression): Export.
* c-parser.h (c_parser_peek_token, c_parser_peek_2nd_token,
c_token_starts_typename, c_parser_next_token_starts_declspecs,
c_parser_next_tokens_start_declaration, c_parser_consume_token,
c_parser_error, c_parser_require, c_parser_skip_until_found,
c_parser_declspecs, c_parser_declarator,
c_parser_cast_expression): Declare.
* gimple-parser.c: Moved from c-parser.c.
* gimple-parser.h (c_parser_parse_gimple_body): Declare.
(c_parser_gimple_pass_list): Likewise.
obj-c/
* config-lang.in (gtfiles): Add c/c-parser.h.
diff --git a/gcc/c/Make-lang.in b/gcc/c/Make-lang.in
index 72c9ae7..cd7108b 100644
--- a/gcc/c/Make-lang.in
+++ b/gcc/c/Make-lang.in
@@ -51,7 +51,8 @@ CFLAGS-c/gccspec.o += $(DRIVER_DEFINES)
# Language-specific object files for C and Objective C.
C_AND_OBJC_OBJS = attribs.o c/c-errors.o c/c-decl.o c/c-typeck.o \
c/c-convert.o c/c-aux-info.o c/c-objc-common.o c/c-parser.o \
- c/c-array-notation.o c/c-fold.o $(C_COMMON_OBJS) $(C_TARGET_OBJS)
+ c/c-array-notation.o c/c-fold.o c/gimple-parser.o \
+ $(C_COMMON_OBJS) $(C_TARGET_OBJS)
# Language-specific object files for C.
C_OBJS = c/c-lang.o c-family/stub-objc.o $(C_AND_OBJC_OBJS)
diff --git a/gcc/c/c-lang.c b/gcc/c/c-lang.c
index b4096d0..a2dd768 100644
--- a/gcc/c/c-lang.c
+++ b/gcc/c/c-lang.c
@@ -25,6 +25,8 @@ along with GCC; see the file COPYING3. If not see
#include "langhooks.h"
#include "langhooks-def.h"
#include "c-objc-common.h"
+#include "c-family/c-pragma.h"
+#include "c-parser.h"
enum c_language_kind c_language = clk_c;
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 639d94f..9b51c48 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -59,18 +59,8 @@ along with GCC; see the file COPYING3. If not see
#include "gimple-expr.h"
#include "context.h"
#include "gcc-rich-location.h"
-#include "tree-vrp.h"
-#include "tree-pass.h"
-#include "tree-pretty-print.h"
-#include "tree.h"
-#include "basic-block.h"
-#include "gimple.h"
-#include "gimple-pretty-print.h"
-#include "tree-ssa.h"
-#include "pass_manager.h"
-#include "tree-ssanames.h"
-#include "gimple-ssa.h"
-#include "tree-dfa.h"
+#include "c-parser.h"
+#include "gimple-parser.h"
/* We need to walk over decls with incomplete struct/union/enum types
after parsing the whole translation unit.
@@ -162,116 +152,6 @@ c_parse_init (void)
}
}
-/* The C lexer intermediates between the lexer in cpplib and c-lex.c
- and the C parser. Unlike the C++ lexer, the parser structure
- stores the lexer information instead of using a separate structure.
- Identifiers are separated into ordinary identifiers, type names,
- keywords and some other Objective-C types of identifiers, and some
- look-ahead is maintained.
-
- ??? It might be a good idea to lex the whole file up front (as for
- C++). It would then be possible to share more of the C and C++
- lexer code, if desired. */
-
-/* More information about the type of a CPP_NAME token. */
-enum c_id_kind {
- /* An ordinary identifier. */
- C_ID_ID,
- /* An identifier declared as a typedef name. */
- C_ID_TYPENAME,
- /* An identifier declared as an Objective-C class name. */
- C_ID_CLASSNAME,
- /* An address space identifier. */
- C_ID_ADDRSPACE,
- /* Not an iden
Re: [PATCH] Fix not caught use-after-scope with -O1 (PR sanitize/78106)
On Tue, Oct 25, 2016 at 02:16:33PM +0200, Martin Liška wrote: > 2016-10-25 Martin Liska > > PR sanitizer/78106 > * sanopt.c (imm_dom_path_with_freeing_call): Handle gasm > statements as they can also contain possibly a freeing call. > > gcc/testsuite/ChangeLog: > > 2016-10-25 Martin Liska > > PR sanitizer/78106 > * gcc.dg/asan/pr78106.c: New test. Ok, thanks. Jakub
[PATCH] Improve vector equality comparisons with SSE4.1 (PR target/78102)
Hi!
SSE4.1 added PCMPEQQ instruction, but only SSE4.2 added PCMPGTQ, and
we've switched _mm_cmpeq_epi64 in r217608 from using __builtin_ia32_*
to generic vector comparison, which works fine for SSE4.2, but not for
SSE4.1. The following patch adds optabs etc. so that we can support
vector equality/non-equality integer comparisons even when other
vector comparisons aren't supported.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2016-10-25 Jakub Jelinek
PR target/78102
* optabs.def (vcondeq_optab, vec_cmpeq_optab): New optabs.
* optabs.c (expand_vec_cond_expr): For comparison codes
EQ_EXPR and NE_EXPR, attempt vcondeq_optab as fallback.
(expand_vec_cmp_expr): For comparison codes
EQ_EXPR and NE_EXPR, attempt vec_cmpeq_optab as fallback.
* optabs-tree.h (expand_vec_cmp_expr_p, expand_vec_cond_expr_p):
Add enum tree_code argument.
* optabs-query.h (get_vec_cmp_eq_icode, get_vcond_eq_icode): New
inline functions.
* optabs-tree.c (expand_vec_cmp_expr_p): Add CODE argument. For
CODE EQ_EXPR or NE_EXPR, attempt to use vec_cmpeq_optab as
fallback.
(expand_vec_cond_expr_p): Add CODE argument. For CODE EQ_EXPR or
NE_EXPR, attempt to use vcondeq_optab as fallback.
* tree-vect-generic.c (expand_vector_comparison,
expand_vector_divmod, expand_vector_condition): Adjust
expand_vec_cmp_expr_p and expand_vec_cond_expr_p callers.
* tree-vect-stmts.c (vectorizable_condition,
vectorizable_comparison): Likewise.
* tree-vect-patterns.c (vect_recog_mixed_size_cond_pattern,
check_bool_pattern, search_type_for_mask_1): Likewise.
* expr.c (do_store_flag): Likewise.
* doc/md.texi (@code{vec_cmpeq@var{m}@var{n}},
@code{vcondeq@var{m}@var{n}}): Document.
* config/i386/sse.md (vec_cmpeqv2div2di, vcondeqv2di):
New expanders.
testsuite/
* gcc.target/i386/pr78102.c: New test.
--- gcc/optabs.def.jj 2016-10-14 12:31:49.0 +0200
+++ gcc/optabs.def 2016-10-25 11:30:13.497467507 +0200
@@ -82,9 +82,11 @@ OPTAB_CD(vec_load_lanes_optab, "vec_load
OPTAB_CD(vec_store_lanes_optab, "vec_store_lanes$a$b")
OPTAB_CD(vcond_optab, "vcond$a$b")
OPTAB_CD(vcondu_optab, "vcondu$a$b")
+OPTAB_CD(vcondeq_optab, "vcondeq$a$b")
OPTAB_CD(vcond_mask_optab, "vcond_mask_$a$b")
OPTAB_CD(vec_cmp_optab, "vec_cmp$a$b")
OPTAB_CD(vec_cmpu_optab, "vec_cmpu$a$b")
+OPTAB_CD(vec_cmpeq_optab, "vec_cmpeq$a$b")
OPTAB_CD(maskload_optab, "maskload$a$b")
OPTAB_CD(maskstore_optab, "maskstore$a$b")
--- gcc/optabs.c.jj 2016-10-17 08:42:34.0 +0200
+++ gcc/optabs.c2016-10-25 11:49:02.800334415 +0200
@@ -5636,7 +5636,12 @@ expand_vec_cond_expr (tree vec_cond_type
icode = get_vcond_icode (mode, cmp_op_mode, unsignedp);
if (icode == CODE_FOR_nothing)
-return 0;
+{
+ if (tcode == EQ_EXPR || tcode == NE_EXPR)
+ icode = get_vcond_eq_icode (mode, cmp_op_mode);
+ if (icode == CODE_FOR_nothing)
+ return 0;
+}
comparison = vector_compare_rtx (tcode, op0a, op0b, unsignedp, icode, 4);
rtx_op1 = expand_normal (op1);
@@ -5675,7 +5680,12 @@ expand_vec_cmp_expr (tree type, tree exp
icode = get_vec_cmp_icode (vmode, mask_mode, unsignedp);
if (icode == CODE_FOR_nothing)
-return 0;
+{
+ if (tcode == EQ_EXPR || tcode == NE_EXPR)
+ icode = get_vec_cmp_eq_icode (vmode, mask_mode);
+ if (icode == CODE_FOR_nothing)
+ return 0;
+}
comparison = vector_compare_rtx (tcode, op0a, op0b, unsignedp, icode, 2);
create_output_operand (&ops[0], target, mask_mode);
--- gcc/optabs-tree.h.jj2016-01-14 19:57:53.0 +0100
+++ gcc/optabs-tree.h 2016-10-25 11:34:29.605262354 +0200
@@ -38,8 +38,8 @@ enum optab_subtype
optab optab_for_tree_code (enum tree_code, const_tree, enum optab_subtype);
bool supportable_convert_operation (enum tree_code, tree, tree, tree *,
enum tree_code *);
-bool expand_vec_cmp_expr_p (tree, tree);
-bool expand_vec_cond_expr_p (tree, tree);
+bool expand_vec_cmp_expr_p (tree, tree, enum tree_code);
+bool expand_vec_cond_expr_p (tree, tree, enum tree_code);
void init_tree_optimization_optabs (tree);
#endif
--- gcc/optabs-query.h.jj 2016-01-04 14:55:51.0 +0100
+++ gcc/optabs-query.h 2016-10-25 11:50:12.448462773 +0200
@@ -90,6 +90,15 @@ get_vec_cmp_icode (machine_mode vmode, m
return convert_optab_handler (tab, vmode, mask_mode);
}
+/* Return insn code for a comparison operator with VMODE
+ resultin MASK_MODE (only for EQ/NE). */
+
+static inline enum insn_code
+get_vec_cmp_eq_icode (machine_mode vmode, machine_mode mask_mode)
+{
+ return convert_optab_handler (vec_cmpeq_optab, vmode, mask_mode);
+}
+
/* Return insn code for a conditional operator with a comparison in
mode CMODE, unsigned if UNS is true, resulting in a value of mo
Re: Implement -Wduplicated-branches (PR c/64279) (v3)
On Mon, Oct 24, 2016 at 04:10:21PM +0200, Marek Polacek wrote:
> On Thu, Oct 20, 2016 at 12:28:36PM +0200, Marek Polacek wrote:
> > I found a problem with this patch--we can't call
> > do_warn_duplicated_branches in
> > build_conditional_expr, because that way the C++-specific codes might leak
> > into
> > the hasher. Instead, I should use operand_equal_p, I think. Let me rework
> > that part of the patch.
>
> Thus:
And one more thing, let's not warn for if { } else { }, either.
Thanks Tobias for testing.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2016-10-25 Marek Polacek
PR c/64279
* c-common.h (do_warn_duplicated_branches_r,
do_warn_duplicated_branches): Declare.
* c-gimplify.c (c_genericize): Walk the function tree calling
do_warn_duplicated_branches_r.
* c-warn.c (expr_from_macro_expansion_r): New.
(do_warn_duplicated_branches): New.
(do_warn_duplicated_branches_r): New.
* c.opt (Wduplicated-branches): New option.
* c-typeck.c (build_conditional_expr): Warn about duplicated branches.
* call.c (build_conditional_expr_1): Warn about duplicated branches.
* semantics.c (finish_expr_stmt): Build statement using the proper
location.
* doc/invoke.texi: Document -Wduplicated-branches.
* fold-const.c (fold_build_cleanup_point_expr): Use the expression
location when building CLEANUP_POINT_EXPR.
* tree.c (add_expr): Handle error_mark_node.
* c-c++-common/Wduplicated-branches-1.c: New test.
* c-c++-common/Wduplicated-branches-2.c: New test.
* c-c++-common/Wduplicated-branches-3.c: New test.
* c-c++-common/Wduplicated-branches-4.c: New test.
* c-c++-common/Wduplicated-branches-5.c: New test.
* c-c++-common/Wduplicated-branches-6.c: New test.
* c-c++-common/Wduplicated-branches-7.c: New test.
* c-c++-common/Wduplicated-branches-8.c: New test.
* c-c++-common/Wduplicated-branches-9.c: New test.
* c-c++-common/Wduplicated-branches-10.c: New test.
* c-c++-common/Wimplicit-fallthrough-7.c: Coalesce dg-warning.
* g++.dg/cpp0x/lambda/lambda-switch.C: Move dg-warning.
* g++.dg/ext/builtin-object-size3.C (bar): Likewise.
* g++.dg/gomp/loop-1.C: Likewise.
* g++.dg/warn/Wduplicated-branches1.C: New test.
* g++.dg/warn/Wduplicated-branches2.C: New test.
diff --git gcc/c-family/c-common.h gcc/c-family/c-common.h
index 547bab2..46e9d2e 100644
--- gcc/c-family/c-common.h
+++ gcc/c-family/c-common.h
@@ -1530,6 +1530,7 @@ extern void maybe_warn_bool_compare (location_t, enum
tree_code, tree, tree);
extern bool maybe_warn_shift_overflow (location_t, tree, tree);
extern void warn_duplicated_cond_add_or_warn (location_t, tree, vec **);
extern bool diagnose_mismatched_attributes (tree, tree);
+extern tree do_warn_duplicated_branches_r (tree *, int *, void *);
/* In c-attribs.c. */
extern bool attribute_takes_identifier_p (const_tree);
diff --git gcc/c-family/c-gimplify.c gcc/c-family/c-gimplify.c
index c18b057..3ed2da8 100644
--- gcc/c-family/c-gimplify.c
+++ gcc/c-family/c-gimplify.c
@@ -125,6 +125,10 @@ c_genericize (tree fndecl)
&pset);
}
+ if (warn_duplicated_branches)
+walk_tree_without_duplicates (&DECL_SAVED_TREE (fndecl),
+ do_warn_duplicated_branches_r, NULL);
+
/* Dump the C-specific tree IR. */
dump_orig = get_dump_info (TDI_original, &local_dump_flags);
if (dump_orig)
diff --git gcc/c-family/c-warn.c gcc/c-family/c-warn.c
index 904f6d3..433f5c3 100644
--- gcc/c-family/c-warn.c
+++ gcc/c-family/c-warn.c
@@ -2154,3 +2154,72 @@ maybe_warn_bool_compare (location_t loc, enum tree_code
code, tree op0,
"with boolean expression is always false", cst);
}
}
+
+/* Callback function to determine whether an expression TP or one of its
+ subexpressions comes from macro expansion. Used to suppress bogus
+ warnings. */
+
+static tree
+expr_from_macro_expansion_r (tree *tp, int *, void *)
+{
+ if (CAN_HAVE_LOCATION_P (*tp)
+ && from_macro_expansion_at (EXPR_LOCATION (*tp)))
+return integer_zero_node;
+
+ return NULL_TREE;
+}
+
+/* Possibly warn when an if-else has identical branches. */
+
+static void
+do_warn_duplicated_branches (tree expr)
+{
+ tree thenb = COND_EXPR_THEN (expr);
+ tree elseb = COND_EXPR_ELSE (expr);
+
+ /* Don't bother if there's no else branch. */
+ if (elseb == NULL_TREE)
+return;
+
+ /* And don't warn for empty statements. */
+ if (TREE_CODE (thenb) == NOP_EXPR
+ && TREE_TYPE (thenb) == void_type_node
+ && TREE_OPERAND (thenb, 0) == size_zero_node)
+return;
+
+ /* ... or empty branches. */
+ if (TREE_CODE (thenb) == STATEMENT_LIST
+ && STATEMENT_LIST_HEAD (thenb) == NULL)
+return;
+
+ /* Compute the hash of the then branch. */
+ inchash::hash hstate0 (0);
+ inchash::ad
Re: [patch, fortran] PR45516 - [F08] allocatable components of recursive type
Hi Paul,
Just one small nit and one idea remaining:
The nit: You've missed one "same_type" at:
> Index: gcc/fortran/trans-array.c
> ===
> *** gcc/fortran/trans-array.c (revision 241467)
> --- gcc/fortran/trans-array.c (working copy)
> *** structure_alloc_comps (gfc_symbol * der_
> *** 8421,8426
> --- 8510,8516
> gfc_add_expr_to_block (&fnblock, tmp);
> }
> else if (c->attr.allocatable && !c->attr.proc_pointer
> + && !(c->ts.type == BT_DERIVED && der_type ==
> c->ts.u.derived) && (!(cmp_has_alloc_comps && c->as)
here
>|| c->attr.codimension))
> {
The idea: One could think about doing the same "same_type" for the chunks in
===
*** gcc/fortran/trans-types.c (revision 241467)
--- gcc/fortran/trans-types.c (working copy)
*** gfc_get_derived_type (gfc_symbol * deriv
which would eliminate evaluating the same checks thrice.
Besides that ok for trunk.
Thanks for the work.
Regards,
Andre
On Tue, 25 Oct 2016 13:40:03 +0200
Paul Richard Thomas wrote:
> Dear Andre,
>
> Thanks for the review. Please find my responses below and the revised
> patch attached
>
> With these changes, OK for trunk?
>
> With respect to Dominique's testcase posted last night at 23:18, this
> is an issue for the feature which I will raise on clf. It seems to me
> that in the circumstances, a deep copy of the allocatable components
> should be made in the FROM expression in move_alloc but I am not sure.
> I will post a PR about this as well, after the patch is committed.
>
> Best regards
>
> Paul
>
> snip
>
> >
> > So recursive types are defined to be simple recursive, i.e., not transitive?
> > type A == uses => type B == uses => type A
>
> This is caught by:
> type(linked_list2), allocatable :: link
>1
> Error: Derived type at (1) has not been previously defined and so
> cannot appear in a derived type definition
> andre.f90:10:28:
>
>type(linked_list1) :: test
> 1
> Error: Symbol ‘test’ at (1) cannot have a type
> andre.f90:12:27:
>
> So, in this implementation anyway, the answer to your question is 'yes'.
>
> snip
>
> >> + if (derived->attr.unlimited_polymorphic
> >> + || derived->attr.abstract
> >> + || !rdt)
> >
> > When unlimited or abstract rdt can never be true. This is redundant here,
> > isn't it?
>
> No. Derived types that are not unlimited or abstract might not have
> recursive components.
>
> snip
>
> >> {
> >> if (!c->attr.pointer && !c->attr.proc_pointer
> >> + && !(c->attr.allocatable && (der == c->ts.u.derived))
> >
> > The inner most pair of parentheses is unnecessary here.
>
> The parentheses have been removed.
>
> snip
>
> > The indentation of the whole block above is wrong. The comment has 6 spaces
> > while it should have 2 only.
>
> Corrected. Well spotted.
>
> snip
>
> > is_derived_type = c->ts.type == BT_DERIVED && der_type == c->ts.u.derived;
> >
> > and using that instead of repeatedly evaluating the expression?
>
> Done. I called it 'same_type'.
> >
> >
> >
> >> *** structure_alloc_comps (gfc_symbol * der_
> >> *** 8137,8142
> >> --- 8141,8222
> >> build_int_cst (TREE_TYPE (comp), 0));
> >> gfc_add_expr_to_block (&tmpblock, tmp);
> >> }
> >> + else if (c->attr.allocatable && !c->attr.codimension)
> >> + {
> >
> > How about putting also the creation of the descriptor into the body that is
> > guarded by the is_allocated, i.e. in pseudo-code make:
>
> snip
>
> done
>
> >
> > Furthermore, why are you always using an array? We do now at code-generation
> > time whether the argument is a scalar or an array, don't we? This is more
> > for my understanding, then an actual comment.
>
> This is to limit the number of deallocators to 1 with a rank=1
> argument. This was the simplest solution.
>
> >
> >> /* The size field is returned as an array index type. Therefore treat
> >> Index: gcc/fortran/trans-types.c
> >> ===
> >> *** gcc/fortran/trans-types.c (revision 241467)
> >> --- gcc/fortran/trans-types.c (working copy)
>
> snip
>
> >> ! && !(c->attr.allocatable && (derived == c->ts.u.derived))
> >
> > Spare parentheses above (derived == ..)
>
> All removed
>
>
> >> Index: gcc/testsuite/gfortran.dg/recursive_alloc_comp_3.f08
> >> ===
> >> *** gcc/testsuite/gfortran.dg/recursive_alloc_comp_3.f08 (revision 0)
> >> --- gcc/testsuite/gfortran.dg/r
Re: [RFC] Speed-up -fprofile-update=atomic
On 10/24/2016 03:51 PM, Richard Biener wrote: > It's quite ad-hoc :/ The IFN will also be a memory optimization > barrier unless you add special support > for it in the alias oracle - so the performance measurement needs to > be taken with a grain of salt > (same is true for all atomics of course... - I have some local patches > to improve things here). Good, thus please ping me with the patches you have and I'll integrate it. > > The way you implement process_sm_for_coverage_counter is more like a > final value replacement. > You could maybe use create_iv for the loop counter or even wind up > computing the final value > (number of iterations) only after the loop, avoiding the IV completely > (eventually SCEV cprop > saves you here afterwards). Or maybe we can basically assign loop->niter as the argument of UPDATE_COVERAGE_COUNTER function? Martin > > Richard.
Fix m68k so it builds with current trunk
It's not currently possible to build various crosses with the current trunk due to warning issues. I suspect this is the first of many patches to address those issues. I'm not familiar with the code in question, but based on Maxim's comment the gcc_unreachable ought to be safe. It could possibly even replace the entire else clause here, but I choose to avoid that simply because it's riskier. I've confirmed the various m68k-* targets from config-all.mk will build after this patch. Installed on the trunk. Jeff diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 6548386..6bbb32c 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,7 @@ +2016-10-25 Jeff Law + + * config/m68k/m68k.c (m68k_get_reloc_decoration): Add gcc_unreachable. + 2016-10-25 Martin Liska PR sanitizer/78106 diff --git a/gcc/config/m68k/m68k.c b/gcc/config/m68k/m68k.c index b152ca8..ce56692 100644 --- a/gcc/config/m68k/m68k.c +++ b/gcc/config/m68k/m68k.c @@ -4550,6 +4550,7 @@ m68k_get_reloc_decoration (enum m68k_reloc reloc) } } } + gcc_unreachable (); case RELOC_TLSGD: return "@TLSGD";
Fix v850 builds with current trunk
Two issues in the v850 port. One is just a comment tweak to make the fallthru bits happy. The other appears to be a real bug caught by the fallthru bits. The v850 has various data sections and we have the ability to place uninitialized commons in each of them. Of course to do that we have to switch to the appropriate section. For something in the TDA section, the code, as written, would switch into the TDA section, then immediately switch to the default bss section because of the missing break. I've confirmed the various v850 targets from config-list.mk build after this patch. Installed on the trunk. Jeff commit aee9c0c19174d8be33a36f1cf45a370a8e3d8134 Author: law Date: Tue Oct 25 14:39:34 2016 + * config/v850/v850.c (v850_handle_data_area_attribute): Fix fallthru comment. (v850_output_aligned_bss): Add missing break. git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@241513 138bc75d-0d04-0410-961f-82ee72b054a4 diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 6bbb32c..9715896 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,5 +1,9 @@ 2016-10-25 Jeff Law + * config/v850/v850.c (v850_handle_data_area_attribute): Fix fallthru + comment. + (v850_output_aligned_bss): Add missing break. + * config/m68k/m68k.c (m68k_get_reloc_decoration): Add gcc_unreachable. 2016-10-25 Martin Liska diff --git a/gcc/config/v850/v850.c b/gcc/config/v850/v850.c index 47344fc..91e182f 100644 --- a/gcc/config/v850/v850.c +++ b/gcc/config/v850/v850.c @@ -2122,7 +2122,7 @@ v850_handle_data_area_attribute (tree* node, *no_add_attrs = true; } - /* Drop through. */ + /* FALLTHRU */ case FUNCTION_DECL: area = v850_get_data_area (decl); @@ -2477,6 +2477,7 @@ v850_output_aligned_bss (FILE * file, case DATA_AREA_TDA: switch_to_section (tdata_section); + break; default: switch_to_section (bss_section);
[PATCH, wwwdocs] Add link to GCC 7 changes.html
Now that we have a GCC 7 changes.html file, shouldn't we make it easy to find? Peter Index: index.html === RCS file: /cvs/gcc/wwwdocs/htdocs/index.html,v retrieving revision 1.1027 diff -u -r1.1027 index.html --- index.html 30 Sep 2016 13:17:26 - 1.1027 +++ index.html 25 Oct 2016 14:40:26 - @@ -161,7 +161,7 @@ Development: - GCC 7.0 (release criteria) + GCC 7.0 (release criteria, changes) Status:
[RFA] Patch to allow SPU port to build with current trunk
The fallthru warnings triggered a couple times in the SPU port. I'm not at all familiar with the chip or the port, but based on the little reading I've done and experience with GCC, I believe this patch is correct. First, there's a missing fallthru comment in spu_sched_reorder for TYPE_LOAD/TYPE_STORE cases. If I'm reading the SPU docs properly a load/store insn is handled by pipe_1 and we're also trying to model some aspects of the load-store unit. So we should be setting pipe_ls and pipe_1: case TYPE_LOAD: case TYPE_STORE: pipe_ls = i; case TYPE_LNOP: case TYPE_SHUF: case TYPE_BR: case TYPE_MULTI1: case TYPE_HBR: pipe_1 = i; break; This looks like intentional fallthru and should just have an appropriate comment to silence the warning. spu_legitimate_address looks far more interesting and I think it's buggy as written: case SUBREG: x = XEXP (x, 0); if (REG_P (x)) return 0; case REG: return INT_REG_OK_FOR_BASE_P (x, reg_ok_strict); I think the test is inverted. We want to consider (subreg (reg)) a valid memory address and reject all other (subreg (...)) expressions. But this code does the opposite. OK for the trunk? Jeff * config/spu/spu.c (spu_sched_reorder): Add missing fallthru comment. (spu_legitimate_address_p): Fix logic error and add missing fallthru comment. diff --git a/gcc/config/spu/spu.c b/gcc/config/spu/spu.c index 5b59b72..820924e 100644 --- a/gcc/config/spu/spu.c +++ b/gcc/config/spu/spu.c @@ -2894,6 +2894,7 @@ spu_sched_reorder (FILE *file ATTRIBUTE_UNUSED, int verbose ATTRIBUTE_UNUSED, case TYPE_LOAD: case TYPE_STORE: pipe_ls = i; + /* FALLTHRU */ case TYPE_LNOP: case TYPE_SHUF: case TYPE_BR: @@ -3532,8 +3533,9 @@ spu_legitimate_address_p (machine_mode mode, case SUBREG: x = XEXP (x, 0); - if (REG_P (x)) + if (!REG_P (x)) return 0; + /* FALLTHRU */ case REG: return INT_REG_OK_FOR_BASE_P (x, reg_ok_strict);
Fix PR78060, PR78061, PR78088
Hi,
these are ICEs by the loop splitting pass. Two problems: using
inconsistent types for operations and not dealing correctly with certain
loop forms.
First (78060 and 78088): I initially used an ideosyncratic way of
calculating the new loop bound, needing several type switches that I
haven't implemented correctly. Reordering the order of evaluations makes
this easier and fixes the ICEs.
Second (78061): I need to know the exit values of all loop phis. In
simple loop forms (where the single exit is after the loop body) that's
easy: it's the same as the value on the backedge. If the exit is in the
middle of a loop that's not the case. This mattered only for virtual ops.
All other values already needed a loop-closed phi-node before or in the
latch block, which made it not empty which rejected the loop form. I've
added a predicate that explicitely checks that the exit values are easily
computable (i.e. that the back edge value is indeed also the exit value).
Together regstrapped on x86-64, all languages + ada. Okay for trunk?
Ciao,
Michael.
PR tree-optimization/78060
PR tree-optimization/78061
PR tree-optimization/78088
* tree-ssa-loop-split.c (easy_exit_values): New function.
(tree_ssa_split_loops): Use it.
(compute_new_first_bound): Change order of operations,
fix invalid use of types.
testsuite:
* g++.dg/pr78060.C: New test.
* gfortran.dg/pr78061.f: New test.
* g++.dg/pr78088.C: New test.
commit 10cbbd81f96d5183979dae647b77ee804179780e
Author: Michael Matz
Date: Mon Oct 24 17:14:08 2016 +0200
pr78060 pr78061 pr78088
diff --git a/gcc/testsuite/g++.dg/pr78060.C b/gcc/testsuite/g++.dg/pr78060.C
new file mode 100644
index 000..d6cc7b3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr78060.C
@@ -0,0 +1,14 @@
+// PR tree-optimization/78060
+// { dg-do compile }
+// { dg-options "-O3 -fsplit-loops" }
+class A {
+public:
+ template int &operator[](T2);
+};
+int a;
+A b;
+void fn1() {
+ long c;
+ for (int l; l < c; ++l)
+b[l] = l < 2 ?: a;
+}
diff --git a/gcc/testsuite/g++.dg/pr78088.C b/gcc/testsuite/g++.dg/pr78088.C
new file mode 100644
index 000..1a5c063
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr78088.C
@@ -0,0 +1,19 @@
+// PR tree-optimization/78088
+// { dg-do compile }
+// { dg-options "-O3 -fsplit-loops" }
+class A {
+public:
+ int m_fn1();
+};
+struct B : A {
+ void m_fn2();
+};
+void B::m_fn2() {
+ long a;
+ int b, c;
+ for (;;) {
+c = 0;
+for (; c < a; ++c, ++b)
+ b > 0 ? m_fn1() : 0;
+ }
+}
diff --git a/gcc/testsuite/gfortran.dg/pr78061.f
b/gcc/testsuite/gfortran.dg/pr78061.f
new file mode 100644
index 000..7e4dd3d
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr78061.f
@@ -0,0 +1,18 @@
+! { dg-do compile }
+! { dg-options "-O3 -fsplit-loops" }
+ SUBROUTINE SSYMM(C)
+ REAL C(LDC,*)
+ LOGICAL LSAME
+ LOGICAL UPPER
+ IF (LSAME) THEN
+ DO 170 J = 1,N
+ DO 140 K = 1,J
+ IF (UPPER) THEN
+ END IF
+ 140 CONTINUE
+ DO 160 K = J + 1,N
+ C(I,J) = B(K)
+ 160 CONTINUE
+ 170 CONTINUE
+ END IF
+ END
diff --git a/gcc/tree-ssa-loop-split.c b/gcc/tree-ssa-loop-split.c
index 53abb36..dac68e6 100644
--- a/gcc/tree-ssa-loop-split.c
+++ b/gcc/tree-ssa-loop-split.c
@@ -190,13 +190,40 @@ find_or_create_guard_phi (struct loop *loop, tree
guard_iv, affine_iv * /*iv*/)
return NULL;
}
+/* Returns true if the exit values of all loop phi nodes can be
+ determined easily (i.e. that connect_loop_phis can determine them). */
+
+static bool
+easy_exit_values (struct loop *loop)
+{
+ edge exit = single_exit (loop);
+ edge latch = loop_latch_edge (loop);
+ gphi_iterator psi;
+
+ /* Currently we regard the exit values as easy if they are the same
+ as the value over the backedge. Which is the case if the definition
+ of the backedge value dominates the exit edge. */
+ for (psi = gsi_start_phis (loop->header); !gsi_end_p (psi); gsi_next (&psi))
+{
+ gphi *phi = psi.phi ();
+ tree next = PHI_ARG_DEF_FROM_EDGE (phi, latch);
+ basic_block bb;
+ if (TREE_CODE (next) == SSA_NAME
+ && (bb = gimple_bb (SSA_NAME_DEF_STMT (next)))
+ && !dominated_by_p (CDI_DOMINATORS, exit->src, bb))
+ return false;
+}
+
+ return true;
+}
+
/* This function updates the SSA form after connect_loops made a new
edge NEW_E leading from LOOP1 exit to LOOP2 (via in intermediate
conditional). I.e. the second loop can now be entered either
via the original entry or via NEW_E, so the entry values of LOOP2
phi nodes are either the original ones or those at the exit
of LOOP1. Insert new phi nodes in LOOP2 pre-header reflecting
- this. */
+ this. The loops need to fulfill easy_exit_values(). */
static void
connect_loop_phis (struct loop *loop1, struct loop *loop2, edge new
Re: [C++ PATCH] RFC: implement P0386R2 - C++17 inline variables
On 21/10/16 16:14, Jakub Jelinek wrote: > On Fri, Oct 21, 2016 at 03:57:34PM +0100, Yao Qi wrote: >> Hi Jakub, >> >> On Thu, Oct 20, 2016 at 5:21 PM, Andre Vieira (lists) >> wrote: >>> <2><8f5>: Abbrev Number: 38 (DW_TAG_member) >>> <8f6> DW_AT_specification: <0x8be> >>> <8fa> DW_AT_linkage_name: (indirect string, offset: 0x4a0): >>> _ZN6BANANA1sE >>> <8fe> DW_AT_location: 5 byte block: 3 64 bf 1 0 >>> (DW_OP_addr: 1bf64) >>> >>> I haven't tested it on other targets. >> >> I can reproduce it on x86_64 as well. > > First of all, I have a pending patch for this area: > http://gcc.gnu.org/ml/gcc-patches/2016-10/msg01183.html > so I think it doesn't really make much sense to discuss it until it gets in. > But unless you are talking about -std=c++17 or code with explicit inline > vars, I don't think anything has really changed in the debug representation > with that patch. > > Jakub > Hi Jakub, I built gcc for the following: 1) revision r241135 2) revision r241135 + cherry-picked your patch in revision r241137 (skipped the patch in revision r241136 because that gives a build failure). 3) trunk + patch in http://gcc.gnu.org/ml/gcc-patches/2016-10/msg01183.html And compiling the member-ptr.cc file in the gdb testsuite without -std=c17 (see https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gdb/testsuite/gdb.cp/member-ptr.cc;h=4b7da34d3a77e3b5c045bd76d1f0a01514a039d7;hb=HEAD) leads to the following behavior: 1) expected behavior, debug of information of objects of 'class A' looks fine. 2) new debug information for objects of 'class A' breaking the test. 3) same as 2) As you can see the file has no explicit inline vars and I do not compile it with -std=c++17. So I'm suspecting your patch changes this behavior in an unexpected way. Regards, Andre
Re: Verify package integrity of downloaded prerequisites (partially fixes 61439)
On 10/24/2016 06:16 PM, Moritz Klammler wrote: Jeff Law writes: On 10/24/2016 02:44 AM, Richard Biener wrote: On Fri, Oct 7, 2016 at 3:10 PM, Moritz Klammler wrote: I would like to bump my patch that makes the `contrib/download_prerequisites` script verify the checksums of the downloaded packages and augments it with a few additional options. All feedback I have received has been incorporated. Is it okay like this? Below is again the latest iteration of the patch. Looks generally ok. Can you remove -eu from the /bin/sh command and leave the copyright in place? The patch is ok with that change. Do you have commit privileges? I'm pretty sure Moritz doesn't have commit privs. So I reverted the gratutious changes to the copyright notice and cobbled together a changlog and committed the result. jeff Thank you very much both of you. Indeed, I do not have commit privileges. In fact, this was my very first patch to be accepted. I did went through the copyright assignment process already, though. Now that the patch is applied, can this bug be closed? https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61439 Done. And ChangeLog updated to include appropriate tags. jeff
Re: [PATCH] Five patches for std::experimental::filesystem
Two more fixes for the filesystem TS, and improved tests.
Handle negative times in filesystem::last_write_time
* src/filesystem/ops.cc
(last_write_time(const path&, file_time_type, error_code&)): Handle
negative times correctly.
* testsuite/experimental/filesystem/operations/last_write_time.cc:
Test writing file times.
Fix error handling in copy_file and equivalent
* src/filesystem/ops.cc (do_copy_file): Report an error if source or
destination is not a regular file (LWG 2712).
(equivalent): Fix error handling and result when only one file exists.
* testsuite/experimental/filesystem/operations/copy.cc: Remove files
created by tests. Test copying directories.
* testsuite/experimental/filesystem/operations/copy_file.cc: Remove
files created by tests.
* testsuite/experimental/filesystem/operations/equivalent.cc: New.
* testsuite/experimental/filesystem/operations/is_empty.cc: New.
* testsuite/experimental/filesystem/operations/read_symlink.cc: Remove
file created by test.
* testsuite/experimental/filesystem/operations/remove_all.cc: New.
* testsuite/util/testsuite_fs.h (~scoped_file): Only try to remove
file if path is non-empty, to support removal by other means.
Tested x86_64-linux, committed to trunk.
commit c58bcab6fc126fa2cf4d3935e21186fa873be980
Author: Jonathan Wakely
Date: Tue Oct 25 00:20:59 2016 +0100
Handle negative times in filesystem::last_write_time
* src/filesystem/ops.cc
(last_write_time(const path&, file_time_type, error_code&)): Handle
negative times correctly.
* testsuite/experimental/filesystem/operations/last_write_time.cc:
Test writing file times.
diff --git a/libstdc++-v3/src/filesystem/ops.cc b/libstdc++-v3/src/filesystem/ops.cc
index fcf747e..32c9c5e 100644
--- a/libstdc++-v3/src/filesystem/ops.cc
+++ b/libstdc++-v3/src/filesystem/ops.cc
@@ -1100,6 +1100,11 @@ fs::last_write_time(const path& p __attribute__((__unused__)),
auto s = chrono::duration_cast(d);
#if _GLIBCXX_USE_UTIMENSAT
auto ns = chrono::duration_cast(d - s);
+ if (ns < ns.zero()) // tv_nsec must be non-negative and less than 10e9.
+{
+ --s;
+ ns += chrono::seconds(1);
+}
struct ::timespec ts[2];
ts[0].tv_sec = 0;
ts[0].tv_nsec = UTIME_OMIT;
diff --git a/libstdc++-v3/testsuite/experimental/filesystem/operations/last_write_time.cc b/libstdc++-v3/testsuite/experimental/filesystem/operations/last_write_time.cc
index dee55a5..74be4f9 100644
--- a/libstdc++-v3/testsuite/experimental/filesystem/operations/last_write_time.cc
+++ b/libstdc++-v3/testsuite/experimental/filesystem/operations/last_write_time.cc
@@ -35,6 +35,8 @@
void
test01()
{
+ // read times
+
using time_type = std::experimental::filesystem::file_time_type;
auto p = __gnu_test::nonexistent_path();
@@ -103,8 +105,46 @@ test01()
#endif
}
+void
+test02()
+{
+ // write times
+
+ using time_type = std::experimental::filesystem::file_time_type;
+
+ __gnu_test::scoped_file f;
+ std::error_code ec;
+ time_type time;
+
+ time = last_write_time(f.path);
+ last_write_time(f.path, time, ec);
+ VERIFY( !ec );
+ VERIFY( last_write_time(f.path) == time );
+
+ time -= std::chrono::milliseconds(1000 * 60 * 10 + 15);
+ last_write_time(f.path, time, ec);
+ VERIFY( !ec );
+ VERIFY( last_write_time(f.path) == time );
+
+ time += std::chrono::milliseconds(1000 * 60 * 20 + 15);
+ last_write_time(f.path, time, ec);
+ VERIFY( !ec );
+ VERIFY( last_write_time(f.path) == time );
+
+ time = time_type();
+ last_write_time(f.path, time, ec);
+ VERIFY( !ec );
+ VERIFY( last_write_time(f.path) == time );
+
+ time -= std::chrono::milliseconds(1000 * 60 * 10 + 15);
+ last_write_time(f.path, time, ec);
+ VERIFY( !ec );
+ VERIFY( last_write_time(f.path) == time );
+}
+
int
main()
{
test01();
+ test02();
}
commit 7115df4e3a4da1f756ca7419a841e5d656cc83f3
Author: Jonathan Wakely
Date: Mon Oct 24 19:52:57 2016 +0100
Fix error handling in copy_file and equivalent
* src/filesystem/ops.cc (do_copy_file): Report an error if source or
destination is not a regular file (LWG 2712).
(equivalent): Fix error handling and result when only one file exists.
* testsuite/experimental/filesystem/operations/copy.cc: Remove files
created by tests. Test copying directories.
* testsuite/experimental/filesystem/operations/copy_file.cc: Remove
files created by tests.
* testsuite/experimental/filesystem/operations/equivalent.cc: New.
* testsuite/experimental/filesystem/operations/is_empty.cc: New.
* testsuite/experimental/filesystem/operations/read_symlink.cc: Remove
file created by test.
* testsuite/experimental/filesystem/operations/remove_all.cc: New.
* testsuite/util/testsuite_fs.h (~scoped_file): Only try to remove
file if path is non-empty, to support removal by othe
Re: PING! Re: [PATCH, Fortran] Extension: COTAN and degree-valued trig intrinsics with -fdec-math
On 10/10/2016 08:06 AM, Fritz Reese wrote:
> --- a/gcc/fortran/intrinsic.texi
> +++ b/gcc/fortran/intrinsic.texi
> @@ -23,6 +23,9 @@ Some basic guidelines for editing this document:
> @end ignore
>
> @tex
> +\gdef\acosd{\mathop{\rm acosd}\nolimits}
> +\gdef\asind{\mathop{\rm asind}\nolimits}
> +\gdef\atand{\mathop{\rm atand}\nolimits}
> \gdef\acos{\mathop{\rm acos}\nolimits}
> \gdef\asin{\mathop{\rm asin}\nolimits}
> \gdef\atan{\mathop{\rm atan}\nolimits}
There should be a new mathop for cosd or else ...
> +@item @emph{Return value}:
> +The return value is of the same type and kind as @var{X}. The real part
> +of the result is in degrees. If @var{X} is of the type @code{REAL},
> +the return value lies in the range @math{ -1 \leq \cosd (x) \leq 1}.
... this will cause texinfo/pdflatex to complain about an undefined
function when you try to run 'make pdf'. The attached adds a mathop for
cosd.
Is this patch OK for trunk?
Cesar
2016-10-25 Cesar Philippidis
gcc/fortran/
* intrinsic.texi (cosd): New mathop.
diff --git a/gcc/fortran/intrinsic.texi b/gcc/fortran/intrinsic.texi
index 16e1d5c..85f781e 100644
--- a/gcc/fortran/intrinsic.texi
+++ b/gcc/fortran/intrinsic.texi
@@ -32,6 +32,7 @@ Some basic guidelines for editing this document:
\gdef\acosh{\mathop{\rm acosh}\nolimits}
\gdef\asinh{\mathop{\rm asinh}\nolimits}
\gdef\atanh{\mathop{\rm atanh}\nolimits}
+\gdef\cosd{\mathop{\rm cosd}\nolimits}
@end tex
Re: PING! Re: [PATCH, Fortran] Extension: COTAN and degree-valued trig intrinsics with -fdec-math
On Tue, Oct 25, 2016 at 11:37 AM Cesar Philippidis
wrote:
>
> On 10/10/2016 08:06 AM, Fritz Reese wrote:
>
> > --- a/gcc/fortran/intrinsic.texi
> > +++ b/gcc/fortran/intrinsic.texi
> > @@ -23,6 +23,9 @@ Some basic guidelines for editing this document:
> > @end ignore
> >
> > @tex
> > +\gdef\acosd{\mathop{\rm acosd}\nolimits}
> > +\gdef\asind{\mathop{\rm asind}\nolimits}
> > +\gdef\atand{\mathop{\rm atand}\nolimits}
> > \gdef\acos{\mathop{\rm acos}\nolimits}
> > \gdef\asin{\mathop{\rm asin}\nolimits}
> > \gdef\atan{\mathop{\rm atan}\nolimits}
>
> There should be a new mathop for cosd or else ...
>
> > +@item @emph{Return value}:
> > +The return value is of the same type and kind as @var{X}. The real part
> > +of the result is in degrees. If @var{X} is of the type @code{REAL},
> > +the return value lies in the range @math{ -1 \leq \cosd (x) \leq 1}.
>
> ... this will cause texinfo/pdflatex to complain about an undefined
> function when you try to run 'make pdf'. The attached adds a mathop for
> cosd.
>
> Is this patch OK for trunk?
>
> Cesar
Yes, I think that counts as 'obvious'. Thanks!
---
Fritz Reese
Re: [PATCH] Improve vector equality comparisons with SSE4.1 (PR target/78102)
On October 25, 2016 3:48:41 PM GMT+02:00, Jakub Jelinek
wrote:
>Hi!
>
>SSE4.1 added PCMPEQQ instruction, but only SSE4.2 added PCMPGTQ, and
>we've switched _mm_cmpeq_epi64 in r217608 from using __builtin_ia32_*
>to generic vector comparison, which works fine for SSE4.2, but not for
>SSE4.1. The following patch adds optabs etc. so that we can support
>vector equality/non-equality integer comparisons even when other
>vector comparisons aren't supported.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Thanks,
Richard.
>2016-10-25 Jakub Jelinek
>
> PR target/78102
> * optabs.def (vcondeq_optab, vec_cmpeq_optab): New optabs.
> * optabs.c (expand_vec_cond_expr): For comparison codes
> EQ_EXPR and NE_EXPR, attempt vcondeq_optab as fallback.
> (expand_vec_cmp_expr): For comparison codes
> EQ_EXPR and NE_EXPR, attempt vec_cmpeq_optab as fallback.
> * optabs-tree.h (expand_vec_cmp_expr_p, expand_vec_cond_expr_p):
> Add enum tree_code argument.
> * optabs-query.h (get_vec_cmp_eq_icode, get_vcond_eq_icode): New
> inline functions.
> * optabs-tree.c (expand_vec_cmp_expr_p): Add CODE argument. For
> CODE EQ_EXPR or NE_EXPR, attempt to use vec_cmpeq_optab as
> fallback.
> (expand_vec_cond_expr_p): Add CODE argument. For CODE EQ_EXPR or
> NE_EXPR, attempt to use vcondeq_optab as fallback.
> * tree-vect-generic.c (expand_vector_comparison,
> expand_vector_divmod, expand_vector_condition): Adjust
> expand_vec_cmp_expr_p and expand_vec_cond_expr_p callers.
> * tree-vect-stmts.c (vectorizable_condition,
> vectorizable_comparison): Likewise.
> * tree-vect-patterns.c (vect_recog_mixed_size_cond_pattern,
> check_bool_pattern, search_type_for_mask_1): Likewise.
> * expr.c (do_store_flag): Likewise.
> * doc/md.texi (@code{vec_cmpeq@var{m}@var{n}},
> @code{vcondeq@var{m}@var{n}}): Document.
> * config/i386/sse.md (vec_cmpeqv2div2di, vcondeqv2di):
> New expanders.
>testsuite/
> * gcc.target/i386/pr78102.c: New test.
>
>--- gcc/optabs.def.jj 2016-10-14 12:31:49.0 +0200
>+++ gcc/optabs.def 2016-10-25 11:30:13.497467507 +0200
>@@ -82,9 +82,11 @@ OPTAB_CD(vec_load_lanes_optab, "vec_load
> OPTAB_CD(vec_store_lanes_optab, "vec_store_lanes$a$b")
> OPTAB_CD(vcond_optab, "vcond$a$b")
> OPTAB_CD(vcondu_optab, "vcondu$a$b")
>+OPTAB_CD(vcondeq_optab, "vcondeq$a$b")
> OPTAB_CD(vcond_mask_optab, "vcond_mask_$a$b")
> OPTAB_CD(vec_cmp_optab, "vec_cmp$a$b")
> OPTAB_CD(vec_cmpu_optab, "vec_cmpu$a$b")
>+OPTAB_CD(vec_cmpeq_optab, "vec_cmpeq$a$b")
> OPTAB_CD(maskload_optab, "maskload$a$b")
> OPTAB_CD(maskstore_optab, "maskstore$a$b")
>
>--- gcc/optabs.c.jj2016-10-17 08:42:34.0 +0200
>+++ gcc/optabs.c 2016-10-25 11:49:02.800334415 +0200
>@@ -5636,7 +5636,12 @@ expand_vec_cond_expr (tree vec_cond_type
>
> icode = get_vcond_icode (mode, cmp_op_mode, unsignedp);
> if (icode == CODE_FOR_nothing)
>-return 0;
>+{
>+ if (tcode == EQ_EXPR || tcode == NE_EXPR)
>+ icode = get_vcond_eq_icode (mode, cmp_op_mode);
>+ if (icode == CODE_FOR_nothing)
>+ return 0;
>+}
>
>comparison = vector_compare_rtx (tcode, op0a, op0b, unsignedp, icode,
>4);
> rtx_op1 = expand_normal (op1);
>@@ -5675,7 +5680,12 @@ expand_vec_cmp_expr (tree type, tree exp
>
> icode = get_vec_cmp_icode (vmode, mask_mode, unsignedp);
> if (icode == CODE_FOR_nothing)
>-return 0;
>+{
>+ if (tcode == EQ_EXPR || tcode == NE_EXPR)
>+ icode = get_vec_cmp_eq_icode (vmode, mask_mode);
>+ if (icode == CODE_FOR_nothing)
>+ return 0;
>+}
>
>comparison = vector_compare_rtx (tcode, op0a, op0b, unsignedp, icode,
>2);
> create_output_operand (&ops[0], target, mask_mode);
>--- gcc/optabs-tree.h.jj 2016-01-14 19:57:53.0 +0100
>+++ gcc/optabs-tree.h 2016-10-25 11:34:29.605262354 +0200
>@@ -38,8 +38,8 @@ enum optab_subtype
>optab optab_for_tree_code (enum tree_code, const_tree, enum
>optab_subtype);
>bool supportable_convert_operation (enum tree_code, tree, tree, tree *,
> enum tree_code *);
>-bool expand_vec_cmp_expr_p (tree, tree);
>-bool expand_vec_cond_expr_p (tree, tree);
>+bool expand_vec_cmp_expr_p (tree, tree, enum tree_code);
>+bool expand_vec_cond_expr_p (tree, tree, enum tree_code);
> void init_tree_optimization_optabs (tree);
>
> #endif
>--- gcc/optabs-query.h.jj 2016-01-04 14:55:51.0 +0100
>+++ gcc/optabs-query.h 2016-10-25 11:50:12.448462773 +0200
>@@ -90,6 +90,15 @@ get_vec_cmp_icode (machine_mode vmode, m
> return convert_optab_handler (tab, vmode, mask_mode);
> }
>
>+/* Return insn code for a comparison operator with VMODE
>+ resultin MASK_MODE (only for EQ/NE). */
>+
>+static inline enum insn_code
>+get_vec_cmp_eq_icode (machine_mode vmode, machine_mode mask_mode)
>+{
>+ return convert_optab_handler (vec_cmpeq_optab, vmode,
Re: [PATCH, Fortran] Four small DEC extensions
On Mon, Oct 24, 2016 at 10:47 PM, Jerry DeLisle wrote:
> On 10/24/2016 07:23 AM, Fritz Reese wrote:
>>
>> https://gcc.gnu.org/ml/fortran/2016-10/msg00087.html et. al.
>>>
>>> On 12/10/16 13:30, Fritz Reese wrote:
Here I submit for review four small extensions to the GNU Fortran
frontend for compatibility with legacy code. I figure it might be a
nice change of pace from my larger patches. Never fear, for I have
more large patches to come, which I will continue to submit
one-at-a-time.
...
>> OK for trunk now? (regtests x86_64-redhat-linux)
>>
>
> God do go!
>
> Jerry
>
Committed r241516, r241517, r241518, r241519.
---
Fritz Reese
---
Fritz Reese
On Mon, Oct 24, 2016 at 10:47 PM, Jerry DeLisle wrote:
> On 10/24/2016 07:23 AM, Fritz Reese wrote:
>>
>> https://gcc.gnu.org/ml/fortran/2016-10/msg00087.html et. al.
>>>
>>> On 12/10/16 13:30, Fritz Reese wrote:
Here I submit for review four small extensions to the GNU Fortran
frontend for compatibility with legacy code. I figure it might be a
nice change of pace from my larger patches. Never fear, for I have
more large patches to come, which I will continue to submit
one-at-a-time.
>>
>>
>> Sorry I took a break from this last week. Back on it this week. I
>> really hope I can get the rest of my major extensions in before the
>> next release stage, which is apparently imminent, so I might have an
>> overwhelming number of submissions this week...
>>
>> Here's how I'm thinking of the flags situation: we use -std=legacy for
>> old compatibility stuff that users shouldn't use, and -fdec for
>> ancient compatibility stuff that users _really_ shouldn't use. So if a
>> user wants the compiler to mimic an old DEC compiler they hit -fdec
>> and get -std=legacy and all the bells and whistles of the
>> deleted/obsolete/compatibility extensions without question. Otherwise
>> they can just use -std=legacy for a more reasonable compilation, or
>> -std=gnu for some such extensions with reasonable warnings.
>>
>> Anyway, here's a resubmission of the several patches for the four
>> extensions mentioned previously, plus an initial cleanup patch for
>> -fdec. Sorry to pork-barrel the cleanup patch in here, but it combines
>> some stuff I've been meaning to touch up with the change of making
>> -fdec into flag_dec using Fortran Var. (This is necessary for the
>> type-print and future extensions to be enabled with -fdec without
>> their own flag.)
>>
>> To summarize, we have
>> 0001 Cleanup -fdec: rolls some options in with fdec that were
>> documented as being a part of it previously (-fdollar-ok,
>> -fcray-pointer, -fd-lines-as-comments, legacy/deleted/obsolete feature
>> standard bits) and moves fdec-structure out of gfc_option to use
>> Fortran Var.
>> 0002 [always] Form feed: feed characters ('\f') accepted as whitespace in
>> source
>> 0003 [-fdec] TYPE is an alias for PRINT
>> 0004 [-std=legacy] %LOC() can be used as an rval (like the LOC()
>> intrinsic)
>> 0005 [-std=legacy] Support for .XOR. operator (same as .NEQV.)
>>
>> The -std=legacy extensions of course compile clean with -std=legacy,
>> give warnings with-std=gnu, and give errors with -std=f*.
>>
>> OK for trunk now? (regtests x86_64-redhat-linux)
>>
>
> God do go!
>
> Jerry
>
Re: [PATCH, Fortran] Four small DEC extensions
On Tue, Oct 25, 2016 at 11:46 AM, Fritz Reese wrote: ... > Committed r241516, r241517, r241518, r241519. ... and r241520. --- Fritz Reese
Re: [Fortran, patch, pr78053, v1] [OOP] SELECT TYPE on CLASS(*) component for deferred length char arrays ICEs for -O > 0
Hi Andre, This patch is fine, apart from s/whose length is no consistently/whose length is not consistently/ in the comment. The testcase in comment #1 of PR78053 is invalid and now give the correct message: type is (character(len=:)) 1 Error: The type-spec at (1) shall specify that each length type parameter is assumed Is this tested anywhere? OK for trunk and, although not a regression, for 6-branch. Cheers Paul On 24 October 2016 at 18:48, Andre Vehreschild wrote: > Hi all, > > attached patch fixes an ICE in gfortran when an unlimited polymorphic entity > was used to store a char array of deferred/assumed length. The patch typedefs > the necessary type now copying the behavior from > trans-array.c::gfc_trans_create_temp_array(). > > Furthermore does the patch now consequently set the _vptr->_size to the > character kind of the char array and the _len component to the length of the > string independent of whether the char array was declared deferred or with a > len given. > > Bootstraps and regtests ok on x86_64-linux/F23. Ok for trunk? > > Regards, > Andre > -- > Andre Vehreschild * Email: vehre ad gmx dot de -- The difference between genius and stupidity is; genius has its limits. Albert Einstein
Re: Fix PR78060, PR78061, PR78088
On October 25, 2016 5:03:15 PM GMT+02:00, Michael Matz wrote:
>Hi,
>
>these are ICEs by the loop splitting pass. Two problems: using
>inconsistent types for operations and not dealing correctly with
>certain
>loop forms.
>
>First (78060 and 78088): I initially used an ideosyncratic way of
>calculating the new loop bound, needing several type switches that I
>haven't implemented correctly. Reordering the order of evaluations
>makes
>this easier and fixes the ICEs.
>
>Second (78061): I need to know the exit values of all loop phis. In
>simple loop forms (where the single exit is after the loop body) that's
>
>easy: it's the same as the value on the backedge. If the exit is in
>the
>middle of a loop that's not the case. This mattered only for virtual
>ops.
>All other values already needed a loop-closed phi-node before or in the
>
>latch block, which made it not empty which rejected the loop form.
>I've
>added a predicate that explicitely checks that the exit values are
>easily
>computable (i.e. that the back edge value is indeed also the exit
>value).
>
>Together regstrapped on x86-64, all languages + ada. Okay for trunk?
OK.
Thanks,
Richard.
>
>Ciao,
>Michael.
>
> PR tree-optimization/78060
> PR tree-optimization/78061
> PR tree-optimization/78088
> * tree-ssa-loop-split.c (easy_exit_values): New function.
> (tree_ssa_split_loops): Use it.
> (compute_new_first_bound): Change order of operations,
> fix invalid use of types.
>
>testsuite:
> * g++.dg/pr78060.C: New test.
> * gfortran.dg/pr78061.f: New test.
> * g++.dg/pr78088.C: New test.
>
>commit 10cbbd81f96d5183979dae647b77ee804179780e
>Author: Michael Matz
>Date: Mon Oct 24 17:14:08 2016 +0200
>
>pr78060 pr78061 pr78088
>
>diff --git a/gcc/testsuite/g++.dg/pr78060.C
>b/gcc/testsuite/g++.dg/pr78060.C
>new file mode 100644
>index 000..d6cc7b3
>--- /dev/null
>+++ b/gcc/testsuite/g++.dg/pr78060.C
>@@ -0,0 +1,14 @@
>+// PR tree-optimization/78060
>+// { dg-do compile }
>+// { dg-options "-O3 -fsplit-loops" }
>+class A {
>+public:
>+ template int &operator[](T2);
>+};
>+int a;
>+A b;
>+void fn1() {
>+ long c;
>+ for (int l; l < c; ++l)
>+b[l] = l < 2 ?: a;
>+}
>diff --git a/gcc/testsuite/g++.dg/pr78088.C
>b/gcc/testsuite/g++.dg/pr78088.C
>new file mode 100644
>index 000..1a5c063
>--- /dev/null
>+++ b/gcc/testsuite/g++.dg/pr78088.C
>@@ -0,0 +1,19 @@
>+// PR tree-optimization/78088
>+// { dg-do compile }
>+// { dg-options "-O3 -fsplit-loops" }
>+class A {
>+public:
>+ int m_fn1();
>+};
>+struct B : A {
>+ void m_fn2();
>+};
>+void B::m_fn2() {
>+ long a;
>+ int b, c;
>+ for (;;) {
>+c = 0;
>+for (; c < a; ++c, ++b)
>+ b > 0 ? m_fn1() : 0;
>+ }
>+}
>diff --git a/gcc/testsuite/gfortran.dg/pr78061.f
>b/gcc/testsuite/gfortran.dg/pr78061.f
>new file mode 100644
>index 000..7e4dd3d
>--- /dev/null
>+++ b/gcc/testsuite/gfortran.dg/pr78061.f
>@@ -0,0 +1,18 @@
>+! { dg-do compile }
>+! { dg-options "-O3 -fsplit-loops" }
>+ SUBROUTINE SSYMM(C)
>+ REAL C(LDC,*)
>+ LOGICAL LSAME
>+ LOGICAL UPPER
>+ IF (LSAME) THEN
>+ DO 170 J = 1,N
>+ DO 140 K = 1,J
>+ IF (UPPER) THEN
>+ END IF
>+ 140 CONTINUE
>+ DO 160 K = J + 1,N
>+ C(I,J) = B(K)
>+ 160 CONTINUE
>+ 170 CONTINUE
>+ END IF
>+ END
>diff --git a/gcc/tree-ssa-loop-split.c b/gcc/tree-ssa-loop-split.c
>index 53abb36..dac68e6 100644
>--- a/gcc/tree-ssa-loop-split.c
>+++ b/gcc/tree-ssa-loop-split.c
>@@ -190,13 +190,40 @@ find_or_create_guard_phi (struct loop *loop, tree
>guard_iv, affine_iv * /*iv*/)
> return NULL;
> }
>
>+/* Returns true if the exit values of all loop phi nodes can be
>+ determined easily (i.e. that connect_loop_phis can determine them).
> */
>+
>+static bool
>+easy_exit_values (struct loop *loop)
>+{
>+ edge exit = single_exit (loop);
>+ edge latch = loop_latch_edge (loop);
>+ gphi_iterator psi;
>+
>+ /* Currently we regard the exit values as easy if they are the same
>+ as the value over the backedge. Which is the case if the
>definition
>+ of the backedge value dominates the exit edge. */
>+ for (psi = gsi_start_phis (loop->header); !gsi_end_p (psi); gsi_next
>(&psi))
>+{
>+ gphi *phi = psi.phi ();
>+ tree next = PHI_ARG_DEF_FROM_EDGE (phi, latch);
>+ basic_block bb;
>+ if (TREE_CODE (next) == SSA_NAME
>+&& (bb = gimple_bb (SSA_NAME_DEF_STMT (next)))
>+&& !dominated_by_p (CDI_DOMINATORS, exit->src, bb))
>+ return false;
>+}
>+
>+ return true;
>+}
>+
> /* This function updates the SSA form after connect_loops made a new
>edge NEW_E leading from LOOP1 exit to LOOP2 (via in intermediate
>conditional). I.e. the second loop can now be entered either
>via the original entry or via NEW_E, so the entry values of LOOP2
>phi nodes are either the original ones or tho
[PATCH, Fortran] Fix initialization of UNIONs with -finit-derived
All,
Currently the '-finit-derived' flag does not handle UNIONs properly.
Specifically, a default initializer may not be appropriately generated
for BT_UNION components, leaving some memory in the middle of the
structure uninitialized even with -finit-derived. This patch ensures
UNION nodes receive a NULL structure constructor with -finit-derived
while simultaneously respecting the first user-given initializers
within the union's MAP nodes, if any. With the NULL structure
constructor the gcc middle-end figures out the full initialization, as
if you were initializing a C union like "u = {0}", and a bit of work
is saved on our end.
The front-end stores an EXPR_NULL node as the UNION's constructor
expression when -finit-derived is given and it has no user-specified
MAP initializers. When the node propagates through the resolution
phase and makes it to trans-expr.c, gfc_trans_subcomponent_assign and
gfc_conv_union_initializer use this to identify whether to generate a
null initializer or use the user-given MAP initializer.
For the attached testcases: dec_init_3.f90 exhibits improper
initialization for unions before this patch, and dec_init_4.f90
verifies that explicit user-given initializers are not regressed with
the implementation.
Since this only touches my own UNION and -finit-derived code I aim to
commit it in a few days if nobody finds anything wrong with the patch.
Bootstraps and regtests on x86_64-redhat-linux.
---
Fritz Reese
2016-10-25 Fritz Reese
Fix initialization of UNIONs with -finit-derived.
gcc/fortran/
* expr.c (generate_union_initializer, get_union_initializer): New.
* expr.c (component_initializer): Consider BT_UNION specially.
* resolve.c (resolve_structure_cons): Hack for BT_UNION.
* trans-expr.c (gfc_trans_subcomponent_assign): Ditto.
* trans-expr.c (gfc_conv_union_initializer): New.
* trans-expr.c (gfc_conv_structure): Replace UNION handling code with
new function gfc_conv_union_initializer.
gcc/testsuite/gfortran.dg/
* dec_init_1.f90, dec_init_2.f90: Remove -fdump-tree-original.
* dec_init_3.f90, dec_init_4.f90: New testcases.
diff --git a/gcc/fortran/expr.c b/gcc/fortran/expr.c
index b3acf1d..1b4b96c 100644
--- a/gcc/fortran/expr.c
+++ b/gcc/fortran/expr.c
@@ -4159,6 +4159,60 @@ gfc_has_default_initializer (gfc_symbol *der)
}
+/*
+ Generate an initializer expression which initializes the entirety of a
union.
+ A normal structure constructor is insufficient without undue effort, because
+ components of maps may be oddly aligned/overlapped. (For example if a
+ character is initialized from one map overtop a real from the other, only
one
+ byte of the real is actually initialized.) Unfortunately we don't know the
+ size of the union right now, so we can't generate a proper initializer, but
+ we use a NULL expr as a placeholder and do the right thing later in
+ gfc_trans_subcomponent_assign.
+ */
+static gfc_expr *
+generate_union_initializer (gfc_component *un)
+{
+ if (un == NULL || un->ts.type != BT_UNION)
+return NULL;
+
+ gfc_expr *placeholder = gfc_get_null_expr (&un->loc);
+ placeholder->ts = un->ts;
+ return placeholder;
+}
+
+
+/* Get the user-specified initializer for a union, if any. This means the user
+ has said to initialize component(s) of a map. For simplicity's sake we
+ only allow the user to initialize the first map. We don't have to worry
+ about overlapping initializers as they are released early in resolution (see
+ resolve_fl_struct). */
+
+static gfc_expr *
+get_union_initializer (gfc_symbol *union_type, gfc_component **map_p)
+{
+ gfc_component *map;
+ gfc_expr *init=NULL;
+
+ if (!union_type || union_type->attr.flavor != FL_UNION)
+return NULL;
+
+ for (map = union_type->components; map; map = map->next)
+{
+ if (gfc_has_default_initializer (map->ts.u.derived))
+{
+ init = gfc_default_initializer (&map->ts);
+ if (map_p)
+*map_p = map;
+ break;
+}
+}
+
+ if (map_p && !init)
+*map_p = NULL;
+
+ return init;
+}
+
/* Fetch or generate an initializer for the given component.
Only generate an initializer if generate is true. */
@@ -4176,6 +4230,43 @@ component_initializer (gfc_typespec *ts, gfc_component
*c, bool generate)
if (c->ts.type == BT_DERIVED || c->ts.type == BT_CLASS)
init = gfc_generate_initializer (&c->ts, true);
+ else if (c->ts.type == BT_UNION && c->ts.u.derived->components)
+{
+ gfc_component *map = NULL;
+ gfc_constructor *ctor;
+ gfc_expr *user_init;
+
+ /* If we don't have a user initializer and we aren't generating one, this
+ union has no initializer. */
+ user_init = get_union_initializer (c->ts.u.derived, &map);
+ if (!user_init && !generate)
+return NULL;
+
+ /* Otherwise use a structure constructor. */
+ init = gfc_get_structure_constructor_
[PATCH, i386]: Backport fixes for PR 78057 and PR 78037 to gcc-6 branch
Hello!
2016-10-25 Uros Bizjak
Backport from mainline
2016-10-21 Jakub Jelinek
PR target/78057
* config/i386/i386.c: Include fold-const-call.h, tree-vrp.h
and tree-ssanames.h.
(ix86_fold_builtin): Fold IX86_BUILTIN_[LT]ZCNT{16,32,64}
with INTEGER_CST argument.
(ix86_gimple_fold_builtin): New function.
(TARGET_GIMPLE_FOLD_BUILTIN): Define.
Backport from mainline
2016-10-20 Uros Bizjak
PR target/78037
* config/i386/bmiintrin.h (__tzcnt_u16): Call __builtin_ia32_tzcnt_u16.
(__tzcnt_u32, _tzcnt_u32): Call __builtin_ia32_tzcnt_u32.
(__tzcnt_u64, _tzcnt_u64): Call __builtin_ia32_tzcnt_u64.
* config/i386/lzcntintrin.h (__lzcnt_u16): Call
__builtin_ia32_lzcnt_u16.
(__lzcnt_u32, _lzcnt_u32): Call __builtin_ia32_lzcnt_u32.
(__lzcnt_u64, _lzcnt_u64): Call __builtin_ia32_lzcnt_u64.
* config/i386/i386.md (UNSPEC_LZCNT, UNSPEC_TZCNT): New unspecs.
(ctz2, *ctz2): Use SWI48 mode iterator.
(bmi_tzcnt_): New expander.
(*bmi_tzcnt__falsedep_1): New define_insn_and_split pattern.
(*bmi_tzcnt__falsedep, *bmi_tzcnt_): New insn patterns.
(clz2_lzcnt, *clz2_lzcnt): Use SWI48 mode iterator.
(lzcnt_): New expander.
(*lzcnt__falsedep_1): New define_insn_and_split pattern.
(*lzcnt__falsedep, *lzcnt_): New insn patterns.
* config/i386/i386-builtin-types.def (UINT_FTYPE_UINT): New.
(UINT64_FTYPE_UINT64): New.
* config/i386/i386-builtin.def (__builtin_clzs): Remove description.
(__builtin_ia32_lzcnt_u16): New description.
(__builtin_ia32_lzcnt_u32): Ditto.
(__builtin_ia32_lzcnt_u64): Ditto.
(__builtin_ctzs): Remove description.
(__builtin_ia32_tzcnt_u16): New description.
(__builtin_ia32_tzcnt_u32): Ditto.
(__builtin_ia32_tzcnt_u64): Ditto.
* config/i386/i386.c (ix86_expand_args_builtin): Handle
UINT_FTYPE_UINT and UINT64_FTYPE_UINT64.
testsuite/ChangeLog:
2016-10-25 Uros Bizjak
* gcc.target/i386/bmi-6.c: XFAIL.
Backport from mainline
2016-10-21 Jakub Jelinek
PR target/78057
* gcc.target/i386/pr78057.c: New test.
Backport from mainline
2016-10-20 Uros Bizjak
PR target/78037
* gcc.target/i386/pr78037.c: New test.
Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.
Please note that I have to XFAIL bmi-6.c due to insufficient inlining
on gcc-6 branch.
If there are no objections, I plan to commit the patch tomorrow.
Uros.
Index: config/i386/bmiintrin.h
===
--- config/i386/bmiintrin.h (revision 241451)
+++ config/i386/bmiintrin.h (working copy)
@@ -37,7 +37,7 @@
extern __inline unsigned short __attribute__((__gnu_inline__,
__always_inline__, __artificial__))
__tzcnt_u16 (unsigned short __X)
{
- return __builtin_ctzs (__X);
+ return __builtin_ia32_tzcnt_u16 (__X);
}
extern __inline unsigned int __attribute__((__gnu_inline__, __always_inline__,
__artificial__))
@@ -97,13 +97,13 @@ _blsr_u32 (unsigned int __X)
extern __inline unsigned int __attribute__((__gnu_inline__, __always_inline__,
__artificial__))
__tzcnt_u32 (unsigned int __X)
{
- return __builtin_ctz (__X);
+ return __builtin_ia32_tzcnt_u32 (__X);
}
extern __inline unsigned int __attribute__((__gnu_inline__, __always_inline__,
__artificial__))
_tzcnt_u32 (unsigned int __X)
{
- return __builtin_ctz (__X);
+ return __builtin_ia32_tzcnt_u32 (__X);
}
@@ -165,13 +165,13 @@ _blsr_u64 (unsigned long long __X)
extern __inline unsigned long long __attribute__((__gnu_inline__,
__always_inline__, __artificial__))
__tzcnt_u64 (unsigned long long __X)
{
- return __builtin_ctzll (__X);
+ return __builtin_ia32_tzcnt_u64 (__X);
}
extern __inline unsigned long long __attribute__((__gnu_inline__,
__always_inline__, __artificial__))
_tzcnt_u64 (unsigned long long __X)
{
- return __builtin_ctzll (__X);
+ return __builtin_ia32_tzcnt_u64 (__X);
}
#endif /* __x86_64__ */
Index: config/i386/i386-builtin-types.def
===
--- config/i386/i386-builtin-types.def (revision 241451)
+++ config/i386/i386-builtin-types.def (working copy)
@@ -201,9 +201,11 @@ DEF_FUNCTION_TYPE (INT, PCCHAR)
DEF_FUNCTION_TYPE (INT64, INT64)
DEF_FUNCTION_TYPE (INT64, V2DF)
DEF_FUNCTION_TYPE (INT64, V4SF)
+DEF_FUNCTION_TYPE (UINT, UINT)
+DEF_FUNCTION_TYPE (UINT16, UINT16)
DEF_FUNCTION_TYPE (UINT64, INT)
-DEF_FUNCTION_TYPE (UINT16, UINT16)
DEF_FUNCTION_TYPE (UINT64, PUNSIGNED)
+DEF_FUNCTION_TYPE (UINT64, UINT64)
DEF_FUNCTION_TYPE (V16QI, PCCHAR)
DEF_FUNCTION_TYPE (V16QI, V16QI)
DEF_FUNCTION_TYPE (V2DF, PCDOUBLE)
Index: config/i386/i386.c
===
--- config/i386/i386.c (revision 241451)
+++ config/i386/i386.c (working copy)
@@ -76,6 +76,8 @@ along with GCC; see the file COPYING3. If not see
#include "case-cfn-macros.h"
#include "regrename.h"
#include
Re: [PATCHv2 1/7, GCC, ARM, V8M] Add support for ARMv8-M's Secure Extensions flag and intrinsics
On 24/08/16 12:00, Andre Vieira (lists) wrote: > On 25/07/16 14:19, Andre Vieira (lists) wrote: >> This patch adds the support of the '-mcmse' option to enable ARMv8-M's >> Security Extensions and supports the following intrinsics: >> cmse_TT >> cmse_TT_fptr >> cmse_TTT >> cmse_TTT_fptr >> cmse_TTA >> cmse_TTA_fptr >> cmse_TTAT >> cmse_TTAT_fptr >> cmse_check_address_range >> cmse_check_pointed_object >> cmse_is_nsfptr >> cmse_nsfptr_create >> >> It also defines the mandatory cmse_address_info struct and the >> __ARM_FEATURE_CMSE macro. >> See Chapter 4, Sections 5.2, 5.3 and 5.6 of ARM®v8-M Security >> Extensions: Requirements on Development Tools >> (http://infocenter.arm.com/help/topic/com.arm.doc.ecm0359818/index.html). >> >> *** gcc/ChangeLog *** >> 2016-07-25 Andre Vieira >> Thomas Preud'homme >> >> * config.gcc (extra_headers): Added arm_cmse.h. >> * config/arm/arm-arches.def (ARM_ARCH): >> (armv8-m): Add FL2_CMSE. >> (armv8-m.main): Likewise. >> (armv8-m.main+dsp): Likewise. >> * config/arm/arm-c.c >> (arm_cpu_builtins): Added __ARM_FEATURE_CMSE macro. >> * config/arm/arm-protos.h >> (arm_is_constant_pool_ref): Define FL2_CMSE. >> * config/arm.c (arm_arch_cmse): New. >> (arm_option_override): New error for unsupported cmse target. >> * config/arm/arm.h (arm_arch_cmse): New. >> * config/arm/arm.opt (mcmse): New. >> * doc/invoke.texi (ARM Options): Add -mcmse. >> * config/arm/arm_cmse.h: New file. >> >> *** libgcc/ChangeLog *** >> 2016-07-25 Andre Vieira >> Thomas Preud'homme >> >> * config/arm/cmse.c: Likewise. >> * config/arm/t-arm (HAVE_CMSE): New. >> >> *** gcc/testsuite/ChangeLog *** >> 2016-07-25 Andre Vieira >> Thomas Preud'homme >> >> * gcc.target/arm/cmse/cmse.exp: New. >> * gcc.target/arm/cmse/cmse-1.c: New. >> * gcc.target/arm/cmse/cmse-12.c: New. >> * lib/target-supports.exp >> (check_effective_target_arm_cmse_ok): New. >> > > Added more documentation as requested. > > This patch adds the support of the '-mcmse' option to enable ARMv8-M's > Security Extensions and supports the following intrinsics: > cmse_TT > cmse_TT_fptr > cmse_TTT > cmse_TTT_fptr > cmse_TTA > cmse_TTA_fptr > cmse_TTAT > cmse_TTAT_fptr > cmse_check_address_range > cmse_check_pointed_object > cmse_is_nsfptr > cmse_nsfptr_create > > It also defines the mandatory cmse_address_info struct and the > __ARM_FEATURE_CMSE macro. > See Chapter 4, Sections 5.2, 5.3 and 5.6 of ARM®v8-M Security > Extensions: Requirements on Development Tools > (http://infocenter.arm.com/help/topic/com.arm.doc.ecm0359818/index.html). > > *** gcc/ChangeLog *** > 2016-07-xx Andre Vieira > Thomas Preud'homme > > * config.gcc (extra_headers): Added arm_cmse.h. > * config/arm/arm-arches.def (ARM_ARCH): > (armv8-m): Add FL2_CMSE. > (armv8-m.main): Likewise. > (armv8-m.main+dsp): Likewise. > * config/arm/arm-c.c > (arm_cpu_builtins): Added __ARM_FEATURE_CMSE macro. > * config/arm/arm-protos.h > (arm_is_constant_pool_ref): Define FL2_CMSE. > * config/arm.c (arm_arch_cmse): New. > (arm_option_override): New error for unsupported cmse target. > * config/arm/arm.h (arm_arch_cmse): New. > * config/arm/arm.opt (mcmse): New. > * doc/invoke.texi (ARM Options): Add -mcmse. > * doc/extend.texi (ARM ARMv8-M Security Extensions): Add section. > * config/arm/arm_cmse.h: New file. > > *** libgcc/ChangeLog *** > 2016-07-xx Andre Vieira > Thomas Preud'homme > * config/arm/cmse.c: Likewise. > * config/arm/t-arm (HAVE_CMSE): New. > > > *** gcc/testsuite/ChangeLog *** > 2016-07-xx Andre Vieira > Thomas Preud'homme > > * gcc.target/arm/cmse/cmse.exp: New. > * gcc.target/arm/cmse/cmse-1.c: New. > * gcc.target/arm/cmse/cmse-12.c: New. > * lib/target-supports.exp > (check_effective_target_arm_cmse_ok): New. > Hi, Rebased previous patch on top of trunk as requested. No changes to ChangeLog. Cheers, Andre diff --git a/gcc/config.gcc b/gcc/config.gcc index 2143d632fd25892e3633abafe17eee9326b1efd8..63245ecec55c1712e8373b31bcf8c655b4ecfcea 100644 --- a/gcc/config.gcc +++ b/gcc/config.gcc @@ -322,7 +322,7 @@ arc*-*-*) arm*-*-*) cpu_type=arm extra_objs="arm-builtins.o aarch-common.o" - extra_headers="mmintrin.h arm_neon.h arm_acle.h arm_fp16.h" + extra_headers="mmintrin.h arm_neon.h arm_acle.h arm_fp16.h arm_cmse.h" target_type_format_char='%' c_target_objs="arm-c.o" cxx_target_objs="arm-c.o" diff --git a/gcc/config/arm/arm-arches.def b/gcc/config/arm/arm-arches.def index 4b196a7d1188de5eca028e5c2597bbc20835201f..1bdcf5d9f92404d5d
Re: [PATCHv2 2/7, GCC, ARM, V8M] Handling ARMv8-M Security Extension's cmse_nonsecure_entry attribute
On 24/08/16 12:00, Andre Vieira (lists) wrote:
> On 25/07/16 14:21, Andre Vieira (lists) wrote:
>> This patch adds support for the ARMv8-M Security Extensions
>> 'cmse_nonsecure_entry' attribute. In this patch we implement the
>> attribute handling and diagnosis around the attribute. See Section 5.4
>> of ARM®v8-M Security Extensions
>> (http://infocenter.arm.com/help/topic/com.arm.doc.ecm0359818/index.html).
>>
>> *** gcc/ChangeLog ***
>> 2016-07-25 Andre Vieira
>> Thomas Preud'homme
>>
>> * config/arm/arm.c (arm_handle_cmse_nonsecure_entry): New.
>> (arm_attribute_table): Added cmse_nonsecure_entry
>> (arm_compute_func_type): Handle cmse_nonsecure_entry.
>> (cmse_func_args_or_return_in_stack): New.
>> (arm_handle_cmse_nonsecure_entry): New.
>> * config/arm/arm.h (ARM_FT_CMSE_ENTRY): New macro define.
>> (IS_CMSE_ENTRY): Likewise.
>>
>> *** gcc/testsuite/ChangeLog ***
>> 2016-07-25 Andre Vieira
>> Thomas Preud'homme
>>
>> * gcc.target/arm/cmse/cmse-3.c: New.
>>
>
> Added more documentation as requested.
>
>
>
> This patch adds support for the ARMv8-M Security Extensions
> 'cmse_nonsecure_entry' attribute. In this patch we implement the
> attribute handling and diagnosis around the attribute. See Section 5.4
> of ARM®v8-M Security Extensions
> (http://infocenter.arm.com/help/topic/com.arm.doc.ecm0359818/index.html).
>
> *** gcc/ChangeLog ***
> 2016-07-xx Andre Vieira
> Thomas Preud'homme
>
> * config/arm/arm.c (arm_handle_cmse_nonsecure_entry): New.
> (arm_attribute_table): Added cmse_nonsecure_entry
> (arm_compute_func_type): Handle cmse_nonsecure_entry.
> (cmse_func_args_or_return_in_stack): New.
> (arm_handle_cmse_nonsecure_entry): New.
> * config/arm/arm.h (ARM_FT_CMSE_ENTRY): New macro define.
> (IS_CMSE_ENTRY): Likewise.
> * doc/extend.texi (ARM ARMv8-M Security Extensions): New attribute.
>
> *** gcc/testsuite/ChangeLog ***
> 2016-07-xx Andre Vieira
> Thomas Preud'homme
>
> * gcc.target/arm/cmse/cmse-3.c: New.
>
Hi,
Rebased previous patch on top of trunk as requested. No changes to
ChangeLog.
Cheers,
Andre
diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index
a370dccdaa9fa4c980c1df11cb95a65cad16ac85..44d1ac45b03dec210e6986f103bf8588119a8aa8
100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -1386,6 +1386,7 @@ enum reg_class
#define ARM_FT_VOLATILE(1 << 4) /* Does not return. */
#define ARM_FT_NESTED (1 << 5) /* Embedded inside another func. */
#define ARM_FT_STACKALIGN (1 << 6) /* Called with misaligned stack. */
+#define ARM_FT_CMSE_ENTRY (1 << 7) /* ARMv8-M non-secure entry function.
*/
/* Some macros to test these flags. */
#define ARM_FUNC_TYPE(t) (t & ARM_FT_TYPE_MASK)
@@ -1394,6 +1395,7 @@ enum reg_class
#define IS_NAKED(t)(t & ARM_FT_NAKED)
#define IS_NESTED(t) (t & ARM_FT_NESTED)
#define IS_STACKALIGN(t) (t & ARM_FT_STACKALIGN)
+#define IS_CMSE_ENTRY(t) (t & ARM_FT_CMSE_ENTRY)
/* Structure used to hold the function stack frame layout. Offsets are
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index
44677c1bccad42c5ad603ea0951d62abcbd6f05d..996f917ae1dc8e16f219984738c3ac3f8b42f09f
100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -135,6 +135,7 @@ static tree arm_handle_isr_attribute (tree *, tree, tree,
int, bool *);
#if TARGET_DLLIMPORT_DECL_ATTRIBUTES
static tree arm_handle_notshared_attribute (tree *, tree, tree, int, bool *);
#endif
+static tree arm_handle_cmse_nonsecure_entry (tree *, tree, tree, int, bool *);
static void arm_output_function_epilogue (FILE *, HOST_WIDE_INT);
static void arm_output_function_prologue (FILE *, HOST_WIDE_INT);
static int arm_comp_type_attributes (const_tree, const_tree);
@@ -347,6 +348,9 @@ static const struct attribute_spec arm_attribute_table[] =
{ "notshared",0, 0, false, true, false, arm_handle_notshared_attribute,
false },
#endif
+ /* ARMv8-M Security Extensions support. */
+ { "cmse_nonsecure_entry", 0, 0, true, false, false,
+arm_handle_cmse_nonsecure_entry, false },
{ NULL, 0, 0, false, false, false, NULL, false }
};
@@ -3662,6 +3666,9 @@ arm_compute_func_type (void)
else
type |= arm_isr_value (TREE_VALUE (a));
+ if (lookup_attribute ("cmse_nonsecure_entry", attr))
+type |= ARM_FT_CMSE_ENTRY;
+
return type;
}
@@ -6661,6 +6668,110 @@ arm_handle_notshared_attribute (tree *node,
}
#endif
+/* This function returns true if a function with declaration FNDECL, name
+ NAME and type FNTYPE uses the stack to pass arguments or return variables
+ and false otherwise. This is used for functions with the attributes
+ 'cmse_nonsecure_call' or 'cmse_nonsecure_entry' and this function will issue
+ di
Re: [PATCH 3/7, GCC, ARM, V8M] ARMv8-M Security Extension's cmse_nonsecure_entry: __acle_se label and bxns return
On 25/07/16 14:23, Andre Vieira (lists) wrote:
> This patch extends support for the ARMv8-M Security Extensions
> 'cmse_nonsecure_entry' attribute in two ways:
>
> 1) Generate two labels for the function, the regular function name and
> one with the function's name appended to '__acle_se_', this will trigger
> the linker to create a secure gateway veneer for this entry function.
> 2) Return from cmse_nonsecure_entry marked functions using bxns.
>
> See Section 5.4 of ARM®v8-M Security Extensions
> (http://infocenter.arm.com/help/topic/com.arm.doc.ecm0359818/index.html).
>
>
> *** gcc/ChangeLog ***
> 2016-07-25 Andre Vieira
> Thomas Preud'homme
>
> * config/arm/arm.c (use_return_insn): Change to return with bxns
> when cmse_nonsecure_entry.
> (output_return_instruction): Likewise.
> (arm_output_function_prologue): Likewise.
> (thumb_pop): Likewise.
> (thumb_exit): Likewise.
> (arm_function_ok_for_sibcall): Disable sibcall for entry functions.
> (arm_asm_declare_function_name): New.
> * config/arm/arm-protos.h (arm_asm_declare_function_name): New.
> * config/arm/elf.h (ASM_DECLARE_FUNCTION_NAME): Redefine to
> use arm_asm_declare_function_name.
>
> *** gcc/testsuite/ChangeLog ***
> 2016-07-25 Andre Vieira
> Thomas Preud'homme
>
> * gcc.target/arm/cmse/cmse-2.c: New.
> * gcc.target/arm/cmse/cmse-4.c: New.
>
Hi,
Rebased previous patch on top of trunk as requested. No changes to
ChangeLog.
Cheers,
Andre
diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h
index
e7d9f824596a62f5c99000940f6190ab6aee9255..a9b8326c2770c1f9a9787743cb5faa549e6b7d02
100644
--- a/gcc/config/arm/arm-protos.h
+++ b/gcc/config/arm/arm-protos.h
@@ -31,6 +31,7 @@ extern int arm_volatile_func (void);
extern void arm_expand_prologue (void);
extern void arm_expand_epilogue (bool);
extern void arm_declare_function_name (FILE *, const char *, tree);
+extern void arm_asm_declare_function_name (FILE *, const char *, tree);
extern void thumb2_expand_return (bool);
extern const char *arm_strip_name_encoding (const char *);
extern void arm_asm_output_labelref (FILE *, const char *);
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index
996f917ae1dc8e16f219984738c3ac3f8b42f09f..bb81e5662e81a26c7d3ccf9f749e8e356e6de35e
100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -3895,6 +3895,11 @@ use_return_insn (int iscond, rtx sibling)
return 0;
}
+ /* ARMv8-M nonsecure entry function need to use bxns to return and thus need
+ several instructions if anything needs to be popped. */
+ if (saved_int_regs && IS_CMSE_ENTRY (func_type))
+return 0;
+
/* If there are saved registers but the LR isn't saved, then we need
two instructions for the return. */
if (saved_int_regs && !(saved_int_regs & (1 << LR_REGNUM)))
@@ -6930,6 +6935,11 @@ arm_function_ok_for_sibcall (tree decl, tree exp)
if (IS_INTERRUPT (func_type))
return false;
+ /* ARMv8-M non-secure entry functions need to return with bxns which is only
+ generated for entry functions themselves. */
+ if (IS_CMSE_ENTRY (arm_current_func_type ()))
+return false;
+
if (!VOID_TYPE_P (TREE_TYPE (DECL_RESULT (cfun->decl
{
/* Check that the return value locations are the same. For
@@ -19765,6 +19775,7 @@ output_return_instruction (rtx operand, bool
really_return, bool reverse,
(e.g. interworking) then we can load the return address
directly into the PC. Otherwise we must load it into LR. */
if (really_return
+ && !IS_CMSE_ENTRY (func_type)
&& (IS_INTERRUPT (func_type) || !TARGET_INTERWORK))
return_reg = reg_names[PC_REGNUM];
else
@@ -19905,8 +19916,10 @@ output_return_instruction (rtx operand, bool
really_return, bool reverse,
break;
default:
+ if (IS_CMSE_ENTRY (func_type))
+ snprintf (instr, sizeof (instr), "bxns%s\t%%|lr", conditional);
/* Use bx if it's available. */
- if (arm_arch5 || arm_arch4t)
+ else if (arm_arch5 || arm_arch4t)
sprintf (instr, "bx%s\t%%|lr", conditional);
else
sprintf (instr, "mov%s\t%%|pc, %%|lr", conditional);
@@ -19919,6 +19932,42 @@ output_return_instruction (rtx operand, bool
really_return, bool reverse,
return "";
}
+/* Output in FILE asm statements needed to declare the NAME of the function
+ defined by its DECL node. */
+
+void
+arm_asm_declare_function_name (FILE *file, const char *name, tree decl)
+{
+ size_t cmse_name_len;
+ char *cmse_name = 0;
+ char cmse_prefix[] = "__acle_se_";
+
+ if (use_cmse && lookup_attribute ("cmse_nonsecure_entry",
+ DECL_ATTRIBUTES (decl)))
+{
+ cmse_name_len = sizeof (cmse_prefix) + strlen (name);
+ cmse_name = XALLOCAVEC (char, cmse_name_len);
+
Re: [PATCHv2 4/7, GCC, ARM, V8M] ARMv8-M Security Extension's cmse_nonsecure_entry: clear registers
On 24/08/16 12:01, Andre Vieira (lists) wrote: > On 25/07/16 14:23, Andre Vieira (lists) wrote: >> This patch extends support for the ARMv8-M Security Extensions >> 'cmse_nonsecure_entry' attribute to safeguard against leak of >> information through unbanked registers. >> >> When returning from a nonsecure entry function we clear all caller-saved >> registers that are not used to pass return values, by writing either the >> LR, in case of general purpose registers, or the value 0, in case of FP >> registers. We use the LR to write to APSR and FPSCR too. We currently do >> not support entry functions that pass arguments or return variables on >> the stack and we diagnose this. This patch relies on the existing code >> to make sure callee-saved registers used in cmse_nonsecure_entry >> functions are saved and restored thus retaining their nonsecure mode >> value, this should be happening already as it is required by AAPCS. >> >> This patch also clears padding bits for cmse_nonsecure_entry functions >> with struct and union return types. For unions a bit is only considered >> a padding bit if it is an unused bit in every field of that union. The >> function that calculates these is used in a later patch to do the same >> for arguments of cmse_nonsecure_call's. >> >> *** gcc/ChangeLog *** >> 2016-07-25 Andre Vieira >> Thomas Preud'homme >> >> * config/arm/arm.c (output_return_instruction): Clear >> registers. >> (thumb2_expand_return): Likewise. >> (thumb1_expand_epilogue): Likewise. >> (thumb_exit): Likewise. >> (arm_expand_epilogue): Likewise. >> (cmse_nonsecure_entry_clear_before_return): New. >> (comp_not_to_clear_mask_str_un): New. >> (compute_not_to_clear_mask): New. >> * config/arm/thumb1.md (*epilogue_insns): Change length attribute. >> * config/arm/thumb2.md (*thumb2_return): Likewise. >> >> *** gcc/testsuite/ChangeLog *** >> 2016-07-25 Andre Vieira >> Thomas Preud'homme >> >> * gcc.target/arm/cmse/cmse.exp: Test different multilibs separate. >> * gcc.target/arm/cmse/struct-1.c: New. >> * gcc.target/arm/cmse/bitfield-1.c: New. >> * gcc.target/arm/cmse/bitfield-2.c: New. >> * gcc.target/arm/cmse/bitfield-3.c: New. >> * gcc.target/arm/cmse/baseline/cmse-2.c: Test that registers are >> cleared. >> * gcc.target/arm/cmse/mainline/soft/cmse-5.c: New. >> * gcc.target/arm/cmse/mainline/hard/cmse-5.c: New. >> * gcc.target/arm/cmse/mainline/hard-sp/cmse-5.c: New. >> * gcc.target/arm/cmse/mainline/softfp/cmse-5.c: New. >> * gcc.target/arm/cmse/mainline/softfp-sp/cmse-5.c: New. >> > > Updated this patch to correctly clear only the cumulative > exception-status (0-4,7) and the condition code bits (28-31) of the > FPSCR. I also adapted the code to be handle the bigger floating point > register files. > > > > This patch extends support for the ARMv8-M Security Extensions > 'cmse_nonsecure_entry' attribute to safeguard against leak of > information through unbanked registers. > > When returning from a nonsecure entry function we clear all caller-saved > registers that are not used to pass return values, by writing either the > LR, in case of general purpose registers, or the value 0, in case of FP > registers. We use the LR to write to APSR. For FPSCR we clear only the > cumulative exception-status (0-4, 7) and the condition code bits > (28-31). We currently do not support entry functions that pass arguments > or return variables on the stack and we diagnose this. This patch relies > on the existing code to make sure callee-saved registers used in > cmse_nonsecure_entry functions are saved and restored thus retaining > their nonsecure mode value, this should be happening already as it is > required by AAPCS. > > This patch also clears padding bits for cmse_nonsecure_entry functions > with struct and union return types. For unions a bit is only considered > a padding bit if it is an unused bit in every field of that union. The > function that calculates these is used in a later patch to do the same > for arguments of cmse_nonsecure_call's. > > *** gcc/ChangeLog *** > 2016-07-xx Andre Vieira > Thomas Preud'homme > > * config/arm/arm.c (output_return_instruction): Clear > registers. > (thumb2_expand_return): Likewise. > (thumb1_expand_epilogue): Likewise. > (thumb_exit): Likewise. > (arm_expand_epilogue): Likewise. > (cmse_nonsecure_entry_clear_before_return): New. > (comp_not_to_clear_mask_str_un): New. > (compute_not_to_clear_mask): New. > * config/arm/thumb1.md (*epilogue_insns): Change length attribute. > * config/arm/thumb2.md (*thumb2_return): Duplicate pattern for > cmse_nonsecure_entry functions. > > *** gcc/testsuite/ChangeLog *** > 2016-07-xx Andre Vieira
Re: [PATCHv2 5/7, GCC, ARM, V8M] Handling ARMv8-M Security Extension's cmse_nonsecure_call attribute
On 24/08/16 12:01, Andre Vieira (lists) wrote:
> On 25/07/16 14:25, Andre Vieira (lists) wrote:
>> This patch adds support for the ARMv8-M Security Extensions
>> 'cmse_nonsecure_call' attribute. This attribute may only be used for
>> function types and when used in combination with the '-mcmse'
>> compilation flag. See Section 5.5 of ARM®v8-M Security Extensions
>> (http://infocenter.arm.com/help/topic/com.arm.doc.ecm0359818/index.html).
>>
>> We currently do not support cmse_nonsecure_call functions that pass
>> arguments or return variables on the stack and we diagnose this.
>>
>> *** gcc/ChangeLog ***
>> 2016-07-25 Andre Vieira
>> Thomas Preud'homme
>>
>> * config/arm/arm.c (gimplify.h): New include.
>> (arm_handle_cmse_nonsecure_call): New.
>> (arm_attribute_table): Added cmse_nonsecure_call.
>>
>> *** gcc/testsuite/ChangeLog ***
>> 2016-07-25 Andre Vieira
>> Thomas Preud'homme
>>
>> * gcc.target/arm/cmse/cmse-3.c: Add tests.
>> * gcc.target/arm/cmse/cmse-4.c: Add tests.
>>
>
> Added more documentation as requested.
>
> ---
>
> This patch adds support for the ARMv8-M Security Extensions
> 'cmse_nonsecure_call' attribute. This attribute may only be used for
> function types and when used in combination with the '-mcmse'
> compilation flag. See Section 5.5 of ARM®v8-M Security Extensions
> (http://infocenter.arm.com/help/topic/com.arm.doc.ecm0359818/index.html).
>
> We currently do not support cmse_nonsecure_call functions that pass
> arguments or return variables on the stack and we diagnose this.
>
> *** gcc/ChangeLog ***
> 2016-07-xx Andre Vieira
> Thomas Preud'homme
>
> * config/arm/arm.c (gimplify.h): New include.
> (arm_handle_cmse_nonsecure_call): New.
> (arm_attribute_table): Added cmse_nonsecure_call.
> * doc/extend.texi (ARM ARMv8-M Security Extensions): New attribute.
>
> *** gcc/testsuite/ChangeLog ***
> 2016-07-xx Andre Vieira
> Thomas Preud'homme
>
> * gcc.target/arm/cmse/cmse-3.c: Add tests.
> * gcc.target/arm/cmse/cmse-4.c: Add tests.
>
Hi,
Rebased previous patch on top of trunk as requested. No changes to
ChangeLog.
Cheers,
Andre
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index
c6260323ecfd2f2842e6a5aab06b67da16619c73..2e8fba85001e9f69eece789990d0b67f1e35bf76
100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -62,6 +62,7 @@
#include "builtins.h"
#include "tm-constrs.h"
#include "rtl-iter.h"
+#include "gimplify.h"
/* This file should be included last. */
#include "target-def.h"
@@ -136,6 +137,7 @@ static tree arm_handle_isr_attribute (tree *, tree, tree,
int, bool *);
static tree arm_handle_notshared_attribute (tree *, tree, tree, int, bool *);
#endif
static tree arm_handle_cmse_nonsecure_entry (tree *, tree, tree, int, bool *);
+static tree arm_handle_cmse_nonsecure_call (tree *, tree, tree, int, bool *);
static void arm_output_function_epilogue (FILE *, HOST_WIDE_INT);
static void arm_output_function_prologue (FILE *, HOST_WIDE_INT);
static int arm_comp_type_attributes (const_tree, const_tree);
@@ -351,6 +353,8 @@ static const struct attribute_spec arm_attribute_table[] =
/* ARMv8-M Security Extensions support. */
{ "cmse_nonsecure_entry", 0, 0, true, false, false,
arm_handle_cmse_nonsecure_entry, false },
+ { "cmse_nonsecure_call", 0, 0, true, false, false,
+arm_handle_cmse_nonsecure_call, false },
{ NULL, 0, 0, false, false, false, NULL, false }
};
@@ -6777,6 +6781,78 @@ arm_handle_cmse_nonsecure_entry (tree *node, tree name,
return NULL_TREE;
}
+
+/* Called upon detection of the use of the cmse_nonsecure_call attribute, this
+ function will check whether the attribute is allowed here and will add the
+ attribute to the function type tree or otherwise issue a diagnostic. The
+ reason we check this at declaration time is to only allow the use of the
+ attribute with declarations of function pointers and not function
+ declarations. This function checks NODE is of the expected type and issues
+ diagnostics otherwise using NAME. If it is not of the expected type
+ *NO_ADD_ATTRS will be set to true. */
+
+static tree
+arm_handle_cmse_nonsecure_call (tree *node, tree name,
+tree /* args */,
+int /* flags */,
+bool *no_add_attrs)
+{
+ tree decl = NULL_TREE;
+ tree type, fntype, main_variant;
+
+ if (!use_cmse)
+{
+ *no_add_attrs = true;
+ return NULL_TREE;
+}
+
+ if (TREE_CODE (*node) == VAR_DECL || TREE_CODE (*node) == TYPE_DECL)
+{
+ decl = *node;
+ type = TREE_TYPE (decl);
+}
+
+ if (!decl
+ || (!(TREE_CODE (type) == POINTER_TYPE
+ && TREE_CODE (TREE_TYPE (type)) == FUNCTION_TYPE)
+ && TREE_CODE (type) != FUNCTION_TYPE))
+{
+ warning
Re: [PATCHv2 6/7, GCC, ARM, V8M] ARMv8-M Security Extension's cmse_nonsecure_call: use __gnu_cmse_nonsecure_call
On 24/08/16 12:01, Andre Vieira (lists) wrote: > On 25/07/16 14:26, Andre Vieira (lists) wrote: >> This patch extends support for the ARMv8-M Security Extensions >> 'cmse_nonsecure_call' to use a new library function >> '__gnu_cmse_nonsecure_call'. This library function is responsible for >> (without using r0-r3 or d0-d7): >> 1) saving and clearing all callee-saved registers using the secure stack >> 2) clearing the LSB of the address passed in r4 and using blxns to >> 'jump' to it >> 3) clearing ASPR, including the 'ge bits' if DSP is enabled >> 4) clearing FPSCR if using non-soft float-abi >> 5) restoring callee-saved registers. >> >> The decisions whether to include DSP 'ge bits' clearing and floating >> point registers (single/double precision) all depends on the multilib used. >> >> See Section 5.5 of ARM®v8-M Security Extensions >> (http://infocenter.arm.com/help/topic/com.arm.doc.ecm0359818/index.html). >> >> *** gcc/ChangeLog *** >> 2016-07-25 Andre Vieira >> Thomas Preud'homme >> >> * config/arm/arm.c (detect_cmse_nonsecure_call): New. >> (cmse_nonsecure_call_clear_caller_saved): New. >> (arm_reorg): Use cmse_nonsecure_call_clear_caller_saved. >> * config/arm/arm-protos.h (detect_cmse_nonsecure_call): New. >> * config/arm/arm.md (call): Handle cmse_nonsecure_entry. >> (call_value): Likewise. >> (nonsecure_call_internal): New. >> (nonsecure_call_value_internal): New. >> * config/arm/thumb1.md (*nonsecure_call_reg_thumb1_v5): New. >> (*nonsecure_call_value_reg_thumb1_v5): New. >> * config/arm/thumb2.md (*nonsecure_call_reg_thumb2): New. >> (*nonsecure_call_value_reg_thumb2): New. >> * config/arm/unspecs.md (UNSPEC_NONSECURE_MEM): New. >> >> *** libgcc/ChangeLog *** >> 2016-07-25 Andre Vieira >> Thomas Preud'homme >> >> * config/arm/cmse_nonsecure_call.S: New. >> * config/arm/t-arm: Compile cmse_nonsecure_call.S >> >> >> *** gcc/testsuite/ChangeLog *** >> 2016-07-25 Andre Vieira >> Thomas Preud'homme >> >> * gcc.target/arm/cmse/cmse.exp: Run tests in mainline dir. >> * gcc.target/arm/cmse/cmse-9.c: Added some extra tests. >> * gcc.target/arm/cmse/baseline/bitfield-4.c: New. >> * gcc.target/arm/cmse/baseline/bitfield-5.c: New. >> * gcc.target/arm/cmse/baseline/bitfield-6.c: New. >> * gcc.target/arm/cmse/baseline/bitfield-7.c: New. >> * gcc.target/arm/cmse/baseline/bitfield-8.c: New. >> * gcc.target/arm/cmse/baseline/bitfield-9.c: New. >> * gcc.target/arm/cmse/baseline/bitfield-and-union-1.c: New. >> * gcc.target/arm/cmse/baseline/cmse-11.c: New. >> * gcc.target/arm/cmse/baseline/cmse-13.c: New. >> * gcc.target/arm/cmse/baseline/cmse-6.c: New. >> * gcc/testsuite/gcc.target/arm/cmse/baseline/union-1.c: New. >> * gcc/testsuite/gcc.target/arm/cmse/baseline/union-2.c: New. >> * gcc.target/arm/cmse/mainline/hard-sp/cmse-13.c: New. >> * gcc.target/arm/cmse/mainline/hard-sp/cmse-7.c: New. >> * gcc.target/arm/cmse/mainline/hard-sp/cmse-8.c: New. >> * gcc.target/arm/cmse/mainline/hard/cmse-13.c: New. >> * gcc.target/arm/cmse/mainline/hard/cmse-7.c: New. >> * gcc.target/arm/cmse/mainline/hard/cmse-8.c: New. >> * gcc.target/arm/cmse/mainline/soft/cmse-13.c: New. >> * gcc.target/arm/cmse/mainline/soft/cmse-7.c: New. >> * gcc.target/arm/cmse/mainline/soft/cmse-8.c: New. >> * gcc.target/arm/cmse/mainline/softfp-sp/cmse-7.c: New. >> * gcc.target/arm/cmse/mainline/softfp-sp/cmse-8.c: New. >> * gcc.target/arm/cmse/mainline/softfp/cmse-13.c: New. >> * gcc.target/arm/cmse/mainline/softfp/cmse-7.c: New. >> * gcc.target/arm/cmse/mainline/softfp/cmse-8.c: New. >> > > Updated this patch to correctly clear only the cumulative > exception-status (0-4,7) and the condition code bits (28-31) of the FPSCR. > > > > This patch extends support for the ARMv8-M Security Extensions > 'cmse_nonsecure_call' to use a new library function > '__gnu_cmse_nonsecure_call'. This library function is responsible for > (without using r0-r3 or d0-d7): > 1) saving and clearing all callee-saved registers using the secure stack > 2) clearing the LSB of the address passed in r4 and using blxns to > 'jump' to it > 3) clearing ASPR, including the 'ge bits' if DSP is enabled > 4) clearing the cumulative exception-status (0-4, 7) and the condition > bits (28-31) of the FPSCR if using non-soft float-abi > 5) restoring callee-saved registers. > > The decisions whether to include DSP 'ge bits' clearing and floating > point registers (single/double precision) all depends on the multilib used. > > See Section 5.5 of ARM®v8-M Security Extensions > (http://infocenter.arm.com/help/topic/com.arm.doc.ecm0359818/index.html). > > *** gcc/ChangeLog *** > 2016-07-xx Andre Vieira > Tho
[PATCH, Fortran] DEC Compatibility: Logical operations on integers become bitwise ops with -fdec
All-
Proposed here is another compatibility extension to the GNU Fortran
frontend which allows logical operations on integer operands with the
-fdec flag. In this case the logical operator is replaced with its
bitwise equivalent.
For example, GNU Fortran by default (correctly) produces an error for
the expression "A .AND. B" when A and/or B are integers. With this
patch when the -fdec legacy compatibility flag is asserted, GNU
Fortran instead silently replaces the expression with "IAND(A, B)",
performing a bitwise-and on the integers A and B. This behavior is to
match the behavior of legacy compilers, and expectations of some
legacy code.
This extension was originally implemented under its own flag
"-fdec-bitwise-ops", but following the recent trend I have just rolled
it into "-fdec". I thought the behavior might be too drastic for
-std=legacy, but if anyone has a different opinion I can certainly do
this with -std=legacy instead of -fdec.
Bootstraps and regtests on x86_64-redhat-linux, OK for trunk?
---
Fritz Reese
From: Fritz Reese
Date: Mon, 24 Oct 2016 13:47:38 -0400
Subject: [PATCH] Logical operations on integers become bitwise ops with -fdec.
gcc/fortran/
* gfortran.texi: Document.
* resolve.c (logical_to_bitwise): New function.
* resolve.c (resolve_operator): Wrap operands with logical_to_bitwise.
gcc/testsuite/gfortran.dg/
* dec_bitwise_ops_1.f90, dec_bitwise_ops_2.f90: New testcases.
---
gcc/fortran/gfortran.texi | 38 ++
gcc/fortran/resolve.c | 105 +++
gcc/testsuite/gfortran.dg/dec_bitwise_ops_1.f90 | 106
gcc/testsuite/gfortran.dg/dec_bitwise_ops_2.f90 | 155 +++
4 files changed, 404 insertions(+), 0 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/dec_bitwise_ops_1.f90
create mode 100644 gcc/testsuite/gfortran.dg/dec_bitwise_ops_2.f90
diff --git a/gcc/fortran/gfortran.texi b/gcc/fortran/gfortran.texi
index 60b619f..0278bd6 100644
--- a/gcc/fortran/gfortran.texi
+++ b/gcc/fortran/gfortran.texi
@@ -1469,6 +1469,7 @@ compatibility extensions along with those enabled by
@option{-std=legacy}.
* TYPE as an alias for PRINT::
* %LOC as an rvalue::
* .XOR. operator::
+* Bitwise logical operators::
@end menu
@node Old-style kind specifications
@@ -2567,6 +2568,43 @@ GNU Fortran supports @code{.XOR.} as a logical operator
with @code{-std=legacy}
for compatibility with legacy code. @code{.XOR.} is equivalent to
@code{.NEQV.}. That is, the output is true if and only if the inputs differ.
+@node Bitwise logical operators
+@subsection Bitwise logical operators
+@cindex logical, bitwise
+
+With @option{-fdec}, GNU Fortran relaxes the type constraints on
+logical operators to allow integer operands, and performs the corresponding
+bitwise operation instead. This flag is for compatibility only, and should be
+avoided in new code. Consider:
+
+@smallexample
+ INTEGER :: i, j
+ i = z'33'
+ j = z'cc'
+ print *, i .AND. j
+@end smallexample
+
+In this example, compiled with @option{-fdec}, GNU Fortran will
+replace the @code{.AND.} operation with a call to the intrinsic
+@code{@ref{IAND}} function, yielding the bitwise-and of @code{i} and @code{j}.
+
+Note that this conversion will occur if at least one operand is of integral
+type. As a result, a logical operand will be converted to an integer when the
+other operand is an integer in a logical operation. In this case,
+@code{.TRUE.} is converted to @code{1} and @code{.FALSE.} to @code{0}.
+
+Here is the mapping of logical operator to bitwise intrinsic used with
+@option{-fdec}:
+
+@multitable @columnfractions .25 .25 .5
+@headitem Operator @tab Intrinsic @tab Bitwise operation
+@item @code{.NOT.} @tab @code{@ref{NOT}} @tab complement
+@item @code{.AND.} @tab @code{@ref{IAND}} @tab intersection
+@item @code{.OR.} @tab @code{@ref{IOR}} @tab union
+@item @code{.NEQV.} @tab @code{@ref{IEOR}} @tab exclusive or
+@item @code{.EQV.} @tab @code{@ref{NOT}(@ref{IEOR})} @tab complement of
exclusive or
+@end multitable
+
@node Extensions not implemented in GNU Fortran
@section Extensions not implemented in GNU Fortran
diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index 4645b57..3c5892b 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -3522,6 +3522,88 @@ compare_shapes (gfc_expr *op1, gfc_expr *op2)
return t;
}
+/* Convert a logical operator to the corresponding bitwise intrinsic call.
+ For example A .AND. B becomes IAND(A, B). */
+static gfc_expr *
+logical_to_bitwise (gfc_expr *e)
+{
+ gfc_expr *tmp, *op1, *op2;
+ gfc_isym_id isym;
+ gfc_actual_arglist *args = NULL;
+
+ gcc_assert (e->expr_type == EXPR_OP);
+
+ isym = GFC_ISYM_NONE;
+ op1 = e->value.op.op1;
+ op2 = e->value.op.op2;
+
+ switch (e->value.op.op)
+{
+case INTRINSIC_NOT:
+ isym = GFC_ISYM_NOT;
+ break;
+case INTRINSIC_AND:
+ isym = GFC_ISYM_IA
Re: [PATCH 7/7, GCC, ARM, V8M] Added support for ARMV8-M Security Extension cmse_nonsecure_caller intrinsic
On 24/08/16 12:01, Andre Vieira (lists) wrote:
> On 25/07/16 14:28, Andre Vieira (lists) wrote:
>> This patch adds support ARMv8-M's Security Extension's
>> cmse_nonsecure_caller intrinsic. This intrinsic is used to check whether
>> an entry function was called from a non-secure state.
>> See Section 5.4.3 of ARM®v8-M Security Extensions: Requirements on
>> Development Tools
>> (http://infocenter.arm.com/help/topic/com.arm.doc.ecm0359818/index.html)
>> for further details.
>>
>> The FIXME in config/arm/arm_cmse.h is for a diagnostic message that is
>> suggested in the ARMv8-M Security Extensions document mentioned above,
>> to diagnose the use of the cmse_nonsecure_caller intrinsic outside of
>> functions with the 'cmse_nonsecure_entry' attribute. Checking whether
>> the intrinsic is called from within such functions can easily be done
>> inside 'arm_expand_builtin'. However, making the warning point to the
>> right location is more complicated. The ARMv8-M Security Extensions
>> specification does mention that such a diagnostic might become
>> mandatory, so I might have to pick this up later, otherwise it is left
>> as a potential extra feature.
>>
>>
>> *** gcc/ChangeLog ***
>> 2016-07-25 Andre Vieira
>> Thomas Preud'homme
>>
>> * config/arm/arm-builtins.c (arm_builtins): Define
>> ARM_BUILTIN_CMSE_NONSECURE_CALLER.
>> (bdesc_2arg): Add line for cmse_nonsecure_caller.
>> (arm_expand_builtin): Handle cmse_nonsecure_caller.
>> * config/arm/arm_cmse.h (cmse_nonsecure_caller): New.
>>
>> *** gcc/testsuite/ChangeLog ***
>> 2016-07-25 Andre Vieira
>> Thomas Preud'homme
>>
>> * gcc.target/arm/cmse/cmse-1.c: Add test for
>> cmse_nonsecure_caller.
>>
> Added more documentation as requested.
>
> ---
>
> This patch adds support ARMv8-M's Security Extension's
> cmse_nonsecure_caller intrinsic. This intrinsic is used to check whether
> an entry function was called from a non-secure state.
> See Section 5.4.3 of ARM®v8-M Security Extensions: Requirements on
> Development Tools
> (http://infocenter.arm.com/help/topic/com.arm.doc.ecm0359818/index.html)
> for further details.
>
> The FIXME in config/arm/arm_cmse.h is for a diagnostic message that is
> suggested in the ARMv8-M Security Extensions document mentioned above,
> to diagnose the use of the cmse_nonsecure_caller intrinsic outside of
> functions with the 'cmse_nonsecure_entry' attribute. Checking whether
> the intrinsic is called from within such functions can easily be done
> inside 'arm_expand_builtin'. However, making the warning point to the
> right location is more complicated. The ARMv8-M Security Extensions
> specification does mention that such a diagnostic might become
> mandatory, so I might have to pick this up later, otherwise it is left
> as a potential extra feature.
>
>
> *** gcc/ChangeLog ***
> 2016-07-xx Andre Vieira
> Thomas Preud'homme
>
> * config/arm/arm-builtins.c (arm_builtins): Define
> ARM_BUILTIN_CMSE_NONSECURE_CALLER.
> (bdesc_2arg): Add line for cmse_nonsecure_caller.
> (arm_expand_builtin): Handle cmse_nonsecure_caller.
> * config/arm/arm_cmse.h (cmse_nonsecure_caller): New.
> * doc/extend.texi (ARM ARMv8-M Security Extensions): New intrinsic.
>
> *** gcc/testsuite/ChangeLog ***
> 2016-07-xx Andre Vieira
> Thomas Preud'homme
>
> * gcc.target/arm/cmse/cmse-1.c: Add test for
> cmse_nonsecure_caller.
>
Hi,
Rebased previous patch on top of trunk as requested. No changes to
ChangeLog.
Cheers,
Andre
diff --git a/gcc/config/arm/arm-builtins.c b/gcc/config/arm/arm-builtins.c
index
e73043db6db69fa64bb1e72cf71a36d7169062db..68fa17f07097bda9f8220d7e86ffd2dd4fdbf169
100644
--- a/gcc/config/arm/arm-builtins.c
+++ b/gcc/config/arm/arm-builtins.c
@@ -528,6 +528,8 @@ enum arm_builtins
ARM_BUILTIN_GET_FPSCR,
ARM_BUILTIN_SET_FPSCR,
+ ARM_BUILTIN_CMSE_NONSECURE_CALLER,
+
#undef CRYPTO1
#undef CRYPTO2
#undef CRYPTO3
@@ -1832,6 +1834,17 @@ arm_init_builtins (void)
= add_builtin_function ("__builtin_arm_stfscr", ftype_set_fpscr,
ARM_BUILTIN_SET_FPSCR, BUILT_IN_MD, NULL,
NULL_TREE);
}
+
+ if (arm_arch_cmse)
+{
+ tree ftype_cmse_nonsecure_caller
+ = build_function_type_list (unsigned_type_node, NULL);
+ arm_builtin_decls[ARM_BUILTIN_CMSE_NONSECURE_CALLER]
+ = add_builtin_function ("__builtin_arm_cmse_nonsecure_caller",
+ ftype_cmse_nonsecure_caller,
+ ARM_BUILTIN_CMSE_NONSECURE_CALLER, BUILT_IN_MD,
+ NULL, NULL_TREE);
+}
}
/* Return the ARM builtin for CODE. */
@@ -2452,6 +2465,12 @@ arm_expand_builtin (tree exp,
emit_insn (pat);
return target;
+case ARM_BUILTIN_CMSE_NONSECURE_CALLER:
+ target = gen_reg_rtx (SImode);
+ op0
RE: Gimple loop splitting v2
Hi Michael, The commit seems to be causing an ICE on aarch64 (just tested latest trunk). I've created a Bugzilla ticket with a test input https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78107 Regards, Tamar > -Original Message- > From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- > ow...@gcc.gnu.org] On Behalf Of Michael Matz > Sent: 24 October 2016 10:02 > To: Bin.Cheng > Cc: Jeff Law; gcc-patches List > Subject: Re: Gimple loop splitting v2 > > Hi, > > On Mon, 24 Oct 2016, Bin.Cheng wrote: > > > Unfortunately I didn't reproduce the improvement in my run on AArch64, > > I will double check if I made some mistakes. > > Yeah, our regular testers also didn't pick up these kinds of improvements. > As I said, the machine was quite jumpy (though not loaded at all, and fixated > to run on one CPU) :-/ > > > Ciao, > Michael.
[PATCH] sched: Do not reorder RTX_FRAME_RELATED_P insns (PR78029)
This patch makes scheduling not reorder any RTX_FRAME_RELATED_P insns
(relative to each other), to fix PR78029. I originally was a bit worried
that this would degrade code quality, but it seems to even improve it:
more other insns are scheduled between the prologue insns.
The problem in PR78029:
We have two insns, in this order:
(insn/f 300 299 267 8 (set (reg:DI 65 lr)
(reg:DI 0 0)) 579 {*movdi_internal64}
(expr_list:REG_DEAD (reg:DI 0 0)
(expr_list:REG_CFA_RESTORE (reg:DI 65 lr)
(nil
...
(insn/f 310 268 134 8 (set (mem/c:DI (plus:DI (reg/f:DI 1 1)
(const_int 144 [0x90])) [6 S8 A8])
(reg:DI 0 0)) 579 {*movdi_internal64}
(expr_list:REG_DEAD (reg:DI 0 0)
(expr_list:REG_CFA_OFFSET (set (mem/c:DI (plus:DI (reg/f:DI 1 1)
(const_int 144 [0x90])) [6 S8 A8])
(reg:DI 65 lr))
(nil
and sched swaps them (when compiling for power6, it tries to put memory
stores together, so insn 310 is moved up past 300 to go together with
some other store). But the REG_CFA_RESTORE and REG_CFA_OFFSET cannot be
swapped (they both say where the orig value of LR now lives).
Tested on powerpc64-linux {-m32,-m64}, no regressions.
Is this okay for trunk?
Segher
2016-10-25 Segher Boessenkool
PR rtl-optimization/78029
* sched-deps.c (sched_analyze_insn): Mark any RTX_FRAME_RELATED_P
insn as depending on the previous one.
(init_deps): Initialize last_frame_related.
* sched-int.h (struct deps_desc): Add last_frame_related field.
---
gcc/sched-deps.c | 13 +
gcc/sched-int.h | 3 +++
2 files changed, 16 insertions(+)
diff --git a/gcc/sched-deps.c b/gcc/sched-deps.c
index 6cd8332..b6341b5 100644
--- a/gcc/sched-deps.c
+++ b/gcc/sched-deps.c
@@ -3502,6 +3502,18 @@ sched_analyze_insn (struct deps_desc *deps, rtx x,
rtx_insn *insn)
if (!deps->readonly)
deps->last_args_size = insn;
}
+
+ /* Prologue and epilogue insns need to stay in the order we have emitted
+ them in. Any attached CFI notes would also be reordered, which can
+ be fatal. This also encourages unrelated insns to be scheduled in
+ between the *logue insns. */
+ if (RTX_FRAME_RELATED_P (insn))
+{
+ if (deps->last_frame_related)
+ add_dependence (insn, deps->last_frame_related, REG_DEP_OUTPUT);
+ if (!deps->readonly)
+ deps->last_frame_related = insn;
+}
}
/* Return TRUE if INSN might not always return normally (e.g. call exit,
@@ -3907,6 +3919,7 @@ init_deps (struct deps_desc *deps, bool lazy_reg_last)
deps->in_post_call_group_p = not_post_call;
deps->last_debug_insn = 0;
deps->last_args_size = 0;
+ deps->last_frame_related = 0;
deps->last_reg_pending_barrier = NOT_A_BARRIER;
deps->readonly = 0;
}
diff --git a/gcc/sched-int.h b/gcc/sched-int.h
index b4a7f92..dd641f2 100644
--- a/gcc/sched-int.h
+++ b/gcc/sched-int.h
@@ -537,6 +537,9 @@ struct deps_desc
/* The last insn bearing REG_ARGS_SIZE that we've seen. */
rtx_insn *last_args_size;
+ /* The last FRAME_RELATED insn that we have seen. */
+ rtx_insn *last_frame_related;
+
/* The maximum register number for the following arrays. Before reload
this is max_reg_num; after reload it is FIRST_PSEUDO_REGISTER. */
int max_reg;
--
1.9.3
[PATCH, GCC/ARM] Optional -mthumb for Thumb only targets
Hi,
Currently when a user compiles for a thumb-only target (such as Cortex-M
processors) without specifying the -mthumb option GCC throws the error "target
CPU does not support ARM mode". This is suboptimal from a usability point of
view: the -mthumb could be deduced from the -march or -mcpu option when there is
no ambiguity.
This patch implements this behavior by extending the DRIVER_SELF_SPECS to
automatically append -mthumb to the command line for thumb-only targets. It does
so by checking the last -march option if any is given or the last -mcpu option
otherwise. There is no ordering issue because conflicting -mcpu and -march is
already handled.
Note that the logic cannot be implemented in function arm_option_override
because we need to provide the modified command line to the GCC driver for
finding the right multilib path and the function arm_option_override is executed
too late for that effect.
ChangeLog entries are as follow:
*** gcc/ChangeLog ***
2016-10-18 Terry Guo
Thomas Preud'homme
PR target/64802
* common/config/arm/arm-common.c (arm_target_thumb_only): New function.
* config/arm/arm-opts.h: Include arm-flags.h.
(struct arm_arch_core_flag): Define.
(arm_arch_core_flags): Define.
* config/arm/arm-protos.h: Include arm-flags.h.
(FL_NONE, FL_ANY, FL_CO_PROC, FL_ARCH3M, FL_MODE26, FL_MODE32,
FL_ARCH4, FL_ARCH5, FL_THUMB, FL_LDSCHED, FL_STRONG, FL_ARCH5E,
FL_XSCALE, FL_ARCH6, FL_VFPV2, FL_WBUF, FL_ARCH6K, FL_THUMB2, FL_NOTM,
FL_THUMB_DIV, FL_VFPV3, FL_NEON, FL_ARCH7EM, FL_ARCH7, FL_ARM_DIV,
FL_ARCH8, FL_CRC32, FL_SMALLMUL, FL_NO_VOLATILE_CE, FL_IWMMXT,
FL_IWMMXT2, FL_ARCH6KZ, FL2_ARCH8_1, FL2_ARCH8_2, FL2_FP16INST,
FL_TUNE, FL_FOR_ARCH2, FL_FOR_ARCH3, FL_FOR_ARCH3M, FL_FOR_ARCH4,
FL_FOR_ARCH4T, FL_FOR_ARCH5, FL_FOR_ARCH5T, FL_FOR_ARCH5E,
FL_FOR_ARCH5TE, FL_FOR_ARCH5TEJ, FL_FOR_ARCH6, FL_FOR_ARCH6J,
FL_FOR_ARCH6K, FL_FOR_ARCH6Z, FL_FOR_ARCH6ZK, FL_FOR_ARCH6KZ,
FL_FOR_ARCH6T2, FL_FOR_ARCH6M, FL_FOR_ARCH7, FL_FOR_ARCH7A,
FL_FOR_ARCH7VE, FL_FOR_ARCH7R, FL_FOR_ARCH7M, FL_FOR_ARCH7EM,
FL_FOR_ARCH8A, FL2_FOR_ARCH8_1A, FL2_FOR_ARCH8_2A, FL_FOR_ARCH8M_BASE,
FL_FOR_ARCH8M_MAIN, arm_feature_set, ARM_FSET_MAKE,
ARM_FSET_MAKE_CPU1, ARM_FSET_MAKE_CPU2, ARM_FSET_CPU1, ARM_FSET_CPU2,
ARM_FSET_EMPTY, ARM_FSET_ANY, ARM_FSET_HAS_CPU1, ARM_FSET_HAS_CPU2,
ARM_FSET_HAS_CPU, ARM_FSET_ADD_CPU1, ARM_FSET_ADD_CPU2,
ARM_FSET_DEL_CPU1, ARM_FSET_DEL_CPU2, ARM_FSET_UNION, ARM_FSET_INTER,
ARM_FSET_XOR, ARM_FSET_EXCLUDE, ARM_FSET_IS_EMPTY,
ARM_FSET_CPU_SUBSET): Move to ...
* config/arm/arm-flags.h: This new file.
* config/arm/arm.h (TARGET_MODE_SPEC_FUNCTIONS): Define.
(EXTRA_SPEC_FUNCTIONS): Add TARGET_MODE_SPEC_FUNCTIONS to its value.
(TARGET_MODE_SPECS): Define.
(DRIVER_SELF_SPECS): Add TARGET_MODE_SPECS to its value.
*** gcc/testsuite/ChangeLog ***
2016-10-11 Thomas Preud'homme
PR target/64802
* gcc.target/arm/optional_thumb-1.c: New test.
* gcc.target/arm/optional_thumb-2.c: New test.
* gcc.target/arm/optional_thumb-3.c: New test.
No regression when running the testsuite for -mcpu=cortex-m0 -mthumb,
-mcpu=cortex-m0 -marm and -mcpu=cortex-a8 -marm
Is this ok for trunk?
Best regards,
Thomas
diff --git a/gcc/common/config/arm/arm-common.c b/gcc/common/config/arm/arm-common.c
index a9abd6b026e2f35844e810fecf09e9890ea41e21..29ae0c35dd036a5293a51dc16f356e6ed668d3c2 100644
--- a/gcc/common/config/arm/arm-common.c
+++ b/gcc/common/config/arm/arm-common.c
@@ -97,6 +97,29 @@ arm_rewrite_mcpu (int argc, const char **argv)
return arm_rewrite_selected_cpu (argv[argc - 1]);
}
+/* Called by the driver to check whether the target denoted by current
+ command line options is a Thumb-only target. ARGV is an array of
+ -march and -mcpu values (ie. it contains the rhs after the equal
+ sign) and we use the last one of them to make a decision. The
+ number of elements in ARGV is given in ARGC. */
+const char *
+arm_target_thumb_only (int argc, const char **argv)
+{
+ unsigned int opt;
+
+ if (argc)
+{
+ for (opt = 0; opt < (ARRAY_SIZE (arm_arch_core_flags)); opt++)
+ if ((strcmp (argv[argc - 1], arm_arch_core_flags[opt].name) == 0)
+ && !ARM_FSET_HAS_CPU1(arm_arch_core_flags[opt].flags, FL_NOTM))
+ return "-mthumb";
+
+ return NULL;
+}
+ else
+return NULL;
+}
+
#undef ARM_CPU_NAME_LENGTH
diff --git a/gcc/config/arm/arm-flags.h b/gcc/config/arm/arm-flags.h
new file mode 100644
index ..9a5991aa07a229a7741e526c2876e7e0e4749db4
--- /dev/null
+++ b/gcc/config/arm/arm-flags.h
@@ -0,0 +1,210 @@
+/* Flags used to identify the presence of processor capabilities.
+
+ Copyright (C) 2016 Free Software Foundation, Inc.
+ Contributed by A
Re: [Fortran, patch, pr78053, v1] [OOP] SELECT TYPE on CLASS(*) component for deferred length char arrays ICEs for -O > 0
Hi Paul, thanks for the review. The typo is corrected and another in the sentences before that, too. Committed as r241528. I will see whether it make sense and is possible to backport to gcc-6. Note the PR78053 is considered a duplicate of pr72770 which now used for tracking. About the testcase for "type is (character(len=:))" there seems to be some difference of where the type is occurs. I think we should investigate further and check whether a bit more is needed. In my trivial testcase: type :: t1 integer :: i = 42 class(t1),pointer :: cp end type class(t1), allocatable :: a select type (a) type is (character(len=:)) ; end select end I get a totally different error message, then the one expected: select_type_1.f03:10:30: ! type is (character(len=:)) 1 Error: Entity '__tmp_CHARACTER_0_1' at (1) has a deferred type parameter and requires either the POINTER or ALLOCATABLE attribute So a bit more of investigation is needed. Again thanks for the review. Regards, Andre On Tue, 25 Oct 2016 17:57:25 +0200 Paul Richard Thomas wrote: > Hi Andre, > > This patch is fine, apart from > s/whose length is no consistently/whose length is not consistently/ > in the comment. > > The testcase in comment #1 of PR78053 is invalid and now give the > correct message: > > type is (character(len=:)) > 1 > Error: The type-spec at (1) shall specify that each length type > parameter is assumed > > Is this tested anywhere? > > OK for trunk and, although not a regression, for 6-branch. > > Cheers > > Paul > > On 24 October 2016 at 18:48, Andre Vehreschild wrote: > > Hi all, > > > > attached patch fixes an ICE in gfortran when an unlimited polymorphic entity > > was used to store a char array of deferred/assumed length. The patch > > typedefs the necessary type now copying the behavior from > > trans-array.c::gfc_trans_create_temp_array(). > > > > Furthermore does the patch now consequently set the _vptr->_size to the > > character kind of the char array and the _len component to the length of the > > string independent of whether the char array was declared deferred or with a > > len given. > > > > Bootstraps and regtests ok on x86_64-linux/F23. Ok for trunk? > > > > Regards, > > Andre > > -- > > Andre Vehreschild * Email: vehre ad gmx dot de > > > -- Andre Vehreschild * Email: vehre ad gmx dot de
[PATCH][AArch64] Improve SHA1 scheduling
SHA1H instructions may be scheduled after a SHA1C instruction
that uses the same input register. However SHA1C updates its input,
so if SHA1H is scheduled after it, it requires an extra move.
Increase the priority of SHA1H to ensure it gets scheduled
earlier, avoiding the move.
Is this something the generic scheduler could do automatically for
instructions with RMW operands?
Passes bootstrap & regress. OK for commit?
ChangeLog:
2016-10-25 Wilco Dijkstra
* config/aarch64/aarch64.c (aarch64_sched_adjust_priority)
New function.
(TARGET_SCHED_ADJUST_PRIORITY): Define target hook.
--
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index
9b2f9cb19343828dc39e9950ebbefe941521942a..2b25bd1bdd6f4e7737f8e04c3b3684cdff6c4b80
100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -13668,6 +13668,26 @@ aarch64_sched_fusion_priority (rtx_insn *insn, int
max_pri,
return;
}
+/* Implement the TARGET_SCHED_ADJUST_PRIORITY hook.
+ Adjust priority of sha1h instructions so they are scheduled before
+ other SHA1 instructions. */
+
+static int
+aarch64_sched_adjust_priority (rtx_insn *insn, int priority)
+{
+ rtx x = PATTERN (insn);
+
+ if (GET_CODE (x) == SET)
+{
+ x = SET_SRC (x);
+
+ if (GET_CODE (x) == UNSPEC && XINT (x, 1) == UNSPEC_SHA1H)
+ return priority + 10;
+}
+
+ return priority;
+}
+
/* Given OPERANDS of consecutive load/store, check if we can merge
them into ldp/stp. LOAD is true if they are load instructions.
MODE is the mode of memory operands. */
@@ -14431,6 +14451,9 @@ aarch64_optab_supported_p (int op, machine_mode mode1,
machine_mode,
#undef TARGET_CAN_USE_DOLOOP_P
#define TARGET_CAN_USE_DOLOOP_P can_use_doloop_if_innermost
+#undef TARGET_SCHED_ADJUST_PRIORITY
+#define TARGET_SCHED_ADJUST_PRIORITY aarch64_sched_adjust_priority
+
#undef TARGET_SCHED_MACRO_FUSION_P
#define TARGET_SCHED_MACRO_FUSION_P aarch64_macro_fusion_p
Re: [PATCH, wwwdocs] Add link to GCC 7 changes.html
On Tue, 25 Oct 2016, Peter Bergner wrote: > Now that we have a GCC 7 changes.html file, shouldn't we make it > easy to find? Good point, thanks. Perhaps add a disclaimer at the top of changes.html that this is still work in progress as part of that commit? Gerald
Re: [PATCH, Fortran] DEC Compatibility: Logical operations on integers become bitwise ops with -fdec
On 10/25/2016 09:24 AM, Fritz Reese wrote: All- Proposed here is another compatibility extension to the GNU Fortran frontend which allows logical operations on integer operands with the -fdec flag. In this case the logical operator is replaced with its bitwise equivalent. For example, GNU Fortran by default (correctly) produces an error for the expression "A .AND. B" when A and/or B are integers. With this patch when the -fdec legacy compatibility flag is asserted, GNU Fortran instead silently replaces the expression with "IAND(A, B)", performing a bitwise-and on the integers A and B. This behavior is to match the behavior of legacy compilers, and expectations of some legacy code. This extension was originally implemented under its own flag "-fdec-bitwise-ops", but following the recent trend I have just rolled it into "-fdec". I thought the behavior might be too drastic for -std=legacy, but if anyone has a different opinion I can certainly do this with -std=legacy instead of -fdec. Bootstraps and regtests on x86_64-redhat-linux, OK for trunk? I think this is OK. -fdec is sufficient Jerry
C++ PATCH for libstdc++ test failure in C++17 mode
Jonathan recently noticed that 20_util/shared_ptr/cons/constexpr.cc
was failing in C++17 mode. This was due to the copy elision changes
introducing TARGET_EXPR that wasn't there before, and
maybe_constant_init deciding that it wasn't sufficiently constant. I
fixed that by looking through the TARGET_EXPR, but that broke
self-assign-test-1.C, because we were optimizing away the
self-assignment of foo and returning the incomplete CONSTRUCTOR that
we were in the process of building up. I fixed that by making sure
that we don't return an incomplete CONSTRUCTOR as the final value of a
constant expression.
Tested x86_64-pc-linux-gnu, applying to trunk.
commit f92fd1ca9864da0259320a8389321aac374c8740
Author: Jason Merrill
Date: Fri Oct 21 23:42:34 2016 -0400
* constexpr.c (maybe_constant_init): Pull out TARGET_EXPR_INITIAL.
(cxx_eval_outermost_constant_expr): Don't return a CONSTRUCTOR
with CONSTRUCTOR_NO_IMPLICIT_ZERO.
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 8f7b7f3..1ebd647 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -1665,6 +1665,10 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree
t,
entry->result = result;
}
+ /* The result of a constexpr function must be completely initialized. */
+ if (TREE_CODE (result) == CONSTRUCTOR)
+CONSTRUCTOR_NO_IMPLICIT_ZERO (result) = false;
+
pop_cx_call_context ();
return unshare_constructor (result);
}
@@ -4483,6 +4487,16 @@ cxx_eval_outermost_constant_expr (tree t, bool
allow_non_constant,
non_constant_p = true;
}
+ if (TREE_CODE (r) == CONSTRUCTOR
+ && CONSTRUCTOR_NO_IMPLICIT_ZERO (r))
+{
+ if (!allow_non_constant)
+ error ("%qE is not a constant expression because it refers to "
+ "an incompletely initialized variable", t);
+ TREE_CONSTANT (r) = false;
+ non_constant_p = true;
+}
+
/* Technically we should check this for all subexpressions, but that
runs into problems with our internal representation of pointer
subtraction and the 5.19 rules are still in flux. */
@@ -4781,6 +4795,8 @@ maybe_constant_init (tree t, tree decl)
t = TREE_OPERAND (t, 0);
if (TREE_CODE (t) == INIT_EXPR)
t = TREE_OPERAND (t, 1);
+ if (TREE_CODE (t) == TARGET_EXPR)
+t = TARGET_EXPR_INITIAL (t);
if (!potential_nondependent_static_init_expression (t))
/* Don't try to evaluate it. */;
else
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-static12.C
b/gcc/testsuite/g++.dg/cpp0x/constexpr-static12.C
new file mode 100644
index 000..4faa8cf
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-static12.C
@@ -0,0 +1,20 @@
+// { dg-do compile { target c++11 } }
+// { dg-final { scan-assembler-not "_ZNSt10unique_ptrC1Ei" } }
+
+namespace std {
+ struct unique_ptr {
+constexpr unique_ptr(int) : p() { }
+~unique_ptr() { }
+void* p;
+ };
+}
+
+void f()
+{
+ static std::unique_ptr p(1);
+}
+
+int main()
+{
+ f();
+}
Re: [PATCH/AARCH64] Handle ILP32 multi-arch
On Fri, Oct 21, 2016 at 1:24 AM, Andrew Pinski wrote: > On Fri, Oct 7, 2016 at 2:08 PM, Andrew Pinski wrote: >> Hi, >> This patch adds ilp32 multi-arch support. This is needed to support >> multi-arch on Debian like systems. >> >> OK? Bootstrapped and tested on aarch64-linux-gnu with no regressions. >> Also tested with ilp32 with a newly built toolchain that supports >> ILP32 with Ubuntu 1604 base. > > Ping? > >> >> Thanks, >> Andrew >> >> ChangeLog: >> * config/aarch64/t-aarch64-linux (MULTILIB_OSDIRNAMES): Handle >> multi-arch for ilp32.
Re: [PATCH, wwwdocs] Add link to GCC 7 changes.html
On 10/25/16 12:17 PM, Gerald Pfeifer wrote: > On Tue, 25 Oct 2016, Peter Bergner wrote: >> Now that we have a GCC 7 changes.html file, shouldn't we make it >> easy to find? > > Good point, thanks. > > Perhaps add a disclaimer at the top of changes.html that this > is still work in progress as part of that commit? Do you mean like the following? If so, we'd have to remember to remove the last hunk when GCC 7 is released. Peter Index: htdocs/index.html === RCS file: /cvs/gcc/wwwdocs/htdocs/index.html,v retrieving revision 1.1027 diff -u -r1.1027 index.html --- htdocs/index.html 30 Sep 2016 13:17:26 - 1.1027 +++ htdocs/index.html 25 Oct 2016 17:48:41 - @@ -161,7 +161,7 @@ Development: - GCC 7.0 (release criteria) + GCC 7.0 (release criteria, changes) Status: Index: htdocs/gcc-7/changes.html === RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-7/changes.html,v retrieving revision 1.20 diff -u -r1.20 changes.html --- htdocs/gcc-7/changes.html 25 Oct 2016 02:04:00 - 1.20 +++ htdocs/gcc-7/changes.html 25 Oct 2016 17:48:41 - @@ -19,6 +19,11 @@ +Disclaimer: GCC 7 has not been released yet, so this document is +a work-in-progress. + + + Caveats GCC now uses https://gcc.gnu.org/wiki/LRAIsDefault";>LRA by
Re: [C++ PATCH] RFC: implement P0386R2 - C++17 inline variables
On Tue, Oct 25, 2016 at 04:05:30PM +0100, Andre Vieira (lists) wrote:
> I built gcc for the following:
> 1) revision r241135
> 2) revision r241135 + cherry-picked your patch in revision r241137
> (skipped the patch in revision r241136 because that gives a build failure).
> 3) trunk + patch in http://gcc.gnu.org/ml/gcc-patches/2016-10/msg01183.html
>
> And compiling the member-ptr.cc file in the gdb testsuite without
> -std=c17 (see
> https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gdb/testsuite/gdb.cp/member-ptr.cc;h=4b7da34d3a77e3b5c045bd76d1f0a01514a039d7;hb=HEAD)
> leads to the following behavior:
>
> 1) expected behavior, debug of information of objects of 'class A' looks
> fine.
> 2) new debug information for objects of 'class A' breaking the test.
> 3) same as 2)
>
> As you can see the file has no explicit inline vars and I do not compile
> it with -std=c++17.
>
> So I'm suspecting your patch changes this behavior in an unexpected way.
I think this patch should fix it, will bootstrap/regtest it now:
2016-10-25 Jakub Jelinek
* dwarf2out.c (gen_member_die): Only reparent_child instead of
splice_child_die if child doesn't have DW_AT_specification attribute.
--- gcc/dwarf2out.c.jj 2016-10-25 19:49:28.0 +0200
+++ gcc/dwarf2out.c 2016-10-25 20:02:33.264639847 +0200
@@ -22624,7 +22624,8 @@ gen_member_die (tree type, dw_die_ref co
/* Handle inline static data members, which only have in-class
declarations. */
if (child->die_tag == DW_TAG_variable
- && child->die_parent == comp_unit_die ())
+ && child->die_parent == comp_unit_die ()
+ && get_AT (child, DW_AT_specification) == NULL)
{
reparent_child (child, context_die);
child->die_tag = DW_TAG_member;
Jakub
[committed] Implement ~line_maps ()
line_maps instances such as the global line_table are
GC-managed, but the htab within location_adhoc_data_map.htab
is not GC-managed.
Previously this was deleted manually by a call to
location_adhoc_data_fini within toplev::main.
However, on adding a call to forcibly_ggc_collect after the
selftests, all of the htabs for the various line_tables
created during the selftests start showing up as leaks
in "make selftest-valgrind", e.g.:
13,536 (1,344 direct, 12,192 indirect) bytes in 12 blocks are definitely lost
in loss record 1,065 of 1,086
at 0x4A081D4: calloc (in
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x16DB3B0: xcalloc (xmalloc.c:163)
by 0x16D8D34: htab_create_typed_alloc (hashtab.c:358)
by 0x16D8DBD: htab_create_alloc (hashtab.c:286)
by 0x16A2CCC: linemap_init(line_maps*, unsigned int) (line-map.c:353)
by 0x1685605:
selftest::line_table_test::line_table_test(selftest::line_table_case const&)
(input.c:1624)
by 0x167D09C:
selftest::test_applying_fixits_modernize_named_init(selftest::line_table_case
const&) (edit-context.c:1430)
by 0x1686827: selftest::for_each_line_table_case(void
(*)(selftest::line_table_case const&)) (input.c:3227)
by 0x167F067: selftest::edit_context_c_tests() (edit-context.c:1658)
by 0x1616E67: selftest::run_tests() (selftest-run-tests.c:71)
by 0xC0DB25: toplev::run_self_tests() (toplev.c:2076)
by 0x618EB4: toplev::main(int, char**) (toplev.c:2153)
This patch removes the manual one-time cleanup in favor of
adding a destructor to class line_maps, which cleans up
the non-GC-managed htab.
Doing so improves "make selftest-valgrind" from:
==61118== LEAK SUMMARY:
==61118==definitely lost: 121,248 bytes in 1,515 blocks
==61118==indirectly lost: 974,344 bytes in 959 blocks
==61118== possibly lost: 0 bytes in 0 blocks
==61118==still reachable: 1,332,599 bytes in 3,684 blocks
==61118== suppressed: 0 bytes in 0 blocks
to:
==57182== LEAK SUMMARY:
==57182==definitely lost: 13,840 bytes in 556 blocks
==57182==indirectly lost: 0 bytes in 0 blocks
==57182== possibly lost: 0 bytes in 0 blocks
==57182==still reachable: 1,355,703 bytes in 3,684 blocks
==57182== suppressed: 0 bytes in 0 blocks
Successfully bootstrapped®rtested on x86_64-pc-linux-gnu.
Committed to trunk as r241533.
gcc/ChangeLog:
* toplev.c (toplev::main): Remove call to
location_adhoc_data_fini.
libcpp/ChangeLog:
* include/line-map.h (line_maps::~line_maps): New dtor.
(location_adhoc_data_fini): Delete decl.
* line-map.c (line_maps::~line_maps): New dtor.
(location_adhoc_data_fini): Delete.
---
gcc/toplev.c | 1 -
libcpp/include/line-map.h | 3 ++-
libcpp/line-map.c | 15 ---
3 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/gcc/toplev.c b/gcc/toplev.c
index 1df80d0..59b84eb 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -2169,7 +2169,6 @@ toplev::main (int argc, char **argv)
diagnostic_finish (global_dc);
finalize_plugins ();
- location_adhoc_data_fini (line_table);
after_memory_report = true;
diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h
index 8be98b5..b61c3cc 100644
--- a/libcpp/include/line-map.h
+++ b/libcpp/include/line-map.h
@@ -699,6 +699,8 @@ struct GTY(()) location_adhoc_data_map {
/* A set of chronological line_map structures. */
struct GTY(()) line_maps {
+
+ ~line_maps ();
maps_info_ordinary info_ordinary;
@@ -977,7 +979,6 @@ LINEMAPS_LAST_ALLOCATED_MACRO_MAP (const line_maps *set)
return (line_map_macro *)LINEMAPS_LAST_ALLOCATED_MAP (set, true);
}
-extern void location_adhoc_data_fini (struct line_maps *);
extern source_location get_combined_adhoc_loc (struct line_maps *,
source_location,
source_range,
diff --git a/libcpp/line-map.c b/libcpp/line-map.c
index c5c42f0..c98ee45 100644
--- a/libcpp/line-map.c
+++ b/libcpp/line-map.c
@@ -57,6 +57,14 @@ static source_location linemap_macro_loc_to_exp_point
(struct line_maps *,
extern unsigned num_expanded_macros_counter;
extern unsigned num_macro_tokens_counter;
+/* Destructor for class line_maps.
+ Ensure non-GC-managed memory is released. */
+
+line_maps::~line_maps ()
+{
+ htab_delete (location_adhoc_data_map.htab);
+}
+
/* Hash function for location_adhoc_data hashtable. */
static hashval_t
@@ -333,13 +341,6 @@ get_pure_location (line_maps *set, source_location loc)
return loc & ~((1 << ordmap->m_range_bits) - 1);
}
-/* Finalize the location_adhoc_data structure. */
-void
-location_adhoc_data_fini (struct line_maps *set)
-{
- htab_delete (set->location_adhoc_data_map.htab);
-}
-
/* Initialize a line map set. */
void
--
1.8.5.3
[PATCH, Fortran] DEC Compatibility: New I/O Specifiers CARRIAGECONTROL, READONLY, SHARE with -fdec
All, Here's the big one. This patch proposes an extension to both the GNU Fortran front-end and runtime library (libgfortran) to support three additional I/O specifiers: CARRIAGECONTROL, READONLY, and SHARE for compatibility with legacy compilers/code. Here's a summary of what the specifiers actually do: CARRIAGECONTROL is to control line termination settings between output records. READONLY implies ACTION='READ' and additionally prevents STATUS='DELETE' from deleting a file on CLOSE. SHARE provides OS-level locks. These specifiers are designed to match the syntax and behavior of legacy compilers. For more info see the attached test cases, source code, and documentation. Each comes with an IOPARM_OPEN_ bit, and CC+SHARE have IOPARM_INQUIRE_ bits. The st_parameter_dt structure needs an additional two bytes to propagate information with CARRIAGECONTROL='FORTRAN'. These two bytes still leave plenty of trailing 'pad' for future expansion. Bootstraps and regtests on x86_64-redhat-linux. There's a fair bit to sift through, so feel free to ask for clarification or provide comments/constructive criticism. OK for trunk? --- Fritz Reese From: Fritz Reese Date: Wed, 5 Oct 2016 20:18:16 -0400 Subject: [PATCH] New I/O specifiers CARRIAGECONTROL, READONLY, SHARE with -fdec gcc/fortran/ * gfortran.texi: Document. * frontend-passes.c (gfc_code_walker): Add SHARE and CARRIAGECONTROL. * io.c (gfc_free_open, gfc_resolve_open, gfc_match_open): Ditto. * gfortran.h (gfc_open): Add SHARE, CARRIAGECONTROL, and READONLY. * io.c (io_tag, match_open_element): Ditto. * ioparm.def: Ditto. * trans-io.c (gfc_trans_open): Ditto. * io.c (match_dec_etag, match_dec_ftag): New functions. libgfortran/io/ * libgfortran.h (IOPARM_OPEN_HAS_READONLY, IOPARM_OPEN_HAS_SHARE, IOPARM_OPEN_HAS_CC): New for READONLY, SHARE, and CARRIAGECONTROL. * close.c (st_close): Support READONLY. * io.h (st_parameter_open, unit_flags): Support SHARE, CARRIAGECONTROL, and READONLY. * open.c (st_open): Ditto. * transfer.c (data_transfer_init): Ditto. * io.h (st_parameter_dt): New member 'cc' for CARRIAGECONTROL. * write.c (write_check_cc, write_cc): New functions for CARRIAGECONTROL. * transfer.c (next_record_cc): Ditto. * file_pos.c (st_endfile): Support SHARE and CARRIAGECONTROL. * io.h (st_parameter_inquire): Ditto. * open.c (edit_modes, new_unit): Ditto. * inquire.c (inquire_via_unit, inquire_via_filename): Ditto. * io.h (unit_share, unit_cc, cc_fortran, IOPARM_INQUIRE_HAS_SHARE, IOPARM_INQUIRE_HAS_CC): New for SHARE and CARRIAGECONTROL. * open.c (share_opt, cc_opt): Ditto. * read.c (read_x): Support CARRIAGECONTROL. * transfer.c (read_sf, next_record_r, next_record_w): Ditto. * write.c (list_formatted_write_scalar, write_a): Ditto. * unix.h (close_share): New prototype. * unix.c (open_share, close_share): New functions to handle SHARE. * unix.c (open_external): Handle READONLY. Call open_share. * close.c (st_close): Call close_share. gcc/testsuite/ * dec_io_1.f90: New test. * dec_io_2.f90: New test. * dec_io_3.f90: New test. * dec_io_4.f90: New test. * dec_io_5.f90: New test. * dec_io_6.f90: New test. --- gcc/fortran/frontend-passes.c |2 + gcc/fortran/gfortran.h |6 +- gcc/fortran/gfortran.texi | 92 - gcc/fortran/io.c | 177 +++- gcc/fortran/ioparm.def |6 + gcc/fortran/trans-io.c | 15 +++ gcc/testsuite/gfortran.dg/dec_io_1.f90 | 101 ++ gcc/testsuite/gfortran.dg/dec_io_2.f90 | 104 +++ gcc/testsuite/gfortran.dg/dec_io_3.f90 | 15 +++ gcc/testsuite/gfortran.dg/dec_io_4.f90 | 17 +++ gcc/testsuite/gfortran.dg/dec_io_5.f90 | 17 +++ gcc/testsuite/gfortran.dg/dec_io_6.f90 | 15 +++ libgfortran/io/close.c | 16 ++- libgfortran/io/file_pos.c |2 + libgfortran/io/inquire.c | 58 +++ libgfortran/io/io.h| 51 + libgfortran/io/open.c | 47 + libgfortran/io/read.c |3 +- libgfortran/io/transfer.c | 59 +-- libgfortran/io/unit.c |6 + libgfortran/io/unix.c | 89 - libgfortran/io/unix.h |3 + libgfortran/io/write.c | 141 +- libgfortran/libgfortran.h |4 + 24 files changed, 1024 insertions(+), 22 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/dec_io_1.f90 create mode 100644 gcc/testsuite/gfortran.dg/dec_io_2.f90 create mode 100644 gcc/testsui
[committed] input.c/libcpp: fix lifetimes of path buffers
Running "make selftest-valgrind" showed various leaks of the form:
408 bytes in 24 blocks are definitely lost in loss record 572 of 679
at 0x4A0645D: malloc (in
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x1B0D057: xmalloc (xmalloc.c:148)
by 0x1ACCAA1: append_file_to_dir(char const*, cpp_dir*) [clone .isra.3]
(files.c:1567)
by 0x1ACD56F: _cpp_find_file (files.c:390)
by 0x1ACF8FB: cpp_read_main_file(cpp_reader*, char const*) (init.c:632)
by 0x1AB3D97: selftest::lexer_test::lexer_test(selftest::line_table_case
const&, char const*, selftest::lexer_test_options*) (input.c:2014)
by 0x1AB792B:
selftest::test_lexer_string_locations_u8(selftest::line_table_case const&)
(input.c:2713)
by 0x1ABA22A: selftest::for_each_line_table_case(void
(*)(selftest::line_table_case const&)) (input.c:3227)
by 0x1ABA381: selftest::input_c_tests() (input.c:3260)
by 0x1A295F1: selftest::run_tests() (selftest-run-tests.c:62)
by 0xF20DC4: toplev::run_self_tests() (toplev.c:2076)
by 0xF20FCD: toplev::main(int, char**) (toplev.c:2153)
Fix the leak by freeing the file->path in destroy_cpp_file.
However, doing so would lead to a use-after-free in input.c's file cache
since the filenames in this cache are the libcpp file->path buffers.
Hence we need to ensure that any references to the file in the input.c
cache are purged before cleaning up file->path. This is normally done
by the temp_source_file dtor. Hence we need to reorder things to that
the temp_source_file dtor runs before cleaning up the cpp_parser. The
patch does this by introducing a wrapper class around cpp_parser *, so
that the dtor can run after the dtor for temp_source_file.
Successfully bootstrapped®rtested on x86_64-pc-linux-gnu.
Committed to trunk as r241536.
gcc/ChangeLog:
* input.c (fcache::file_patch): Add comment about lifetime.
(selftest::cpp_reader_ptr): New class.
(selftest::lexer_test): Convert m_parser from cpp_reader *
to a cpp_reader_ptr, and move m_tempfile to after it.
(selftest::lexer_test::lexer_test): Update for above reordering.
(lexer_test::~lexer_test): Move cleanup of m_parser to
cpp_reader_ptr's dtor.
libcpp/ChangeLog:
* files.c (destroy_cpp_file): Free file->path.
---
gcc/input.c| 47 +++
libcpp/files.c | 1 +
2 files changed, 40 insertions(+), 8 deletions(-)
diff --git a/gcc/input.c b/gcc/input.c
index 6131659..c2042e8 100644
--- a/gcc/input.c
+++ b/gcc/input.c
@@ -63,6 +63,10 @@ struct fcache
array. */
unsigned use_count;
+ /* The file_path is the key for identifying a particular file in
+ the cache.
+ For libcpp-using code, the underlying buffer for this field is
+ owned by the corresponding _cpp_file within the cpp_reader. */
const char *file_path;
FILE *fp;
@@ -1918,6 +1922,29 @@ class lexer_test_options
virtual void apply (lexer_test &) = 0;
};
+/* Wrapper around an cpp_reader *, which calls cpp_finish and cpp_destroy
+ in its dtor.
+
+ This is needed by struct lexer_test to ensure that the cleanup of the
+ cpp_reader happens *after* the cleanup of the temp_source_file. */
+
+class cpp_reader_ptr
+{
+ public:
+ cpp_reader_ptr (cpp_reader *ptr) : m_ptr (ptr) {}
+
+ ~cpp_reader_ptr ()
+ {
+cpp_finish (m_ptr, NULL);
+cpp_destroy (m_ptr);
+ }
+
+ operator cpp_reader * () const { return m_ptr; }
+
+ private:
+ cpp_reader *m_ptr;
+};
+
/* A struct for writing lexer tests. */
struct lexer_test
@@ -1928,9 +1955,16 @@ struct lexer_test
const cpp_token *get_token ();
- temp_source_file m_tempfile;
+ /* The ordering of these fields matters.
+ The line_table_test must be first, since the cpp_reader_ptr
+ uses it.
+ The cpp_reader must be cleaned up *after* the temp_source_file
+ since the filenames in input.c's input cache are owned by the
+ cpp_reader; in particular, when ~temp_source_file evicts the
+ filename the filenames must still be alive. */
line_table_test m_ltt;
- cpp_reader *m_parser;
+ cpp_reader_ptr m_parser;
+ temp_source_file m_tempfile;
string_concat_db m_concats;
};
@@ -1998,11 +2032,11 @@ ebcdic_execution_charset
*ebcdic_execution_charset::s_singleton;
start parsing the tempfile. */
lexer_test::lexer_test (const line_table_case &case_, const char *content,
- lexer_test_options *options) :
+ lexer_test_options *options)
+: m_ltt (case_),
+ m_parser (cpp_create_reader (CLK_GNUC99, NULL, line_table)),
/* Create a tempfile and write the text to it. */
m_tempfile (SELFTEST_LOCATION, ".c", content),
- m_ltt (case_),
- m_parser (cpp_create_reader (CLK_GNUC99, NULL, line_table)),
m_concats ()
{
if (options)
@@ -2026,9 +2060,6 @@ lexer_test::~lexer_test ()
tok = cpp_get_token_with_location (m_parser, &loc);
ASSERT_NE (tok, NULL);
ASSERT_EQ (tok->type, CPP_EOF);
-
- cpp_finish (m_par
Re: [PATCH] 77864 Fix noexcept conditions for map/set default constructors
On 24/10/2016 13:03, Jonathan Wakely wrote:
On 12/10/16 22:36 +0200, François Dumont wrote:
On 10/10/2016 23:01, Tim Song wrote:
Trying again...with a few edits.
On Mon, Oct 10, 2016 at 3:24 PM, François Dumont
wrote:
@@ -602,24 +612,32 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
struct _Rb_tree_impl : public _Node_allocator
{
_Key_compare _M_key_compare;
- _Rb_tree_node_base _M_header;
+ _Rb_header_node _M_header;
+#if __cplusplus < 201103L
size_type _M_node_count; // Keeps track of size of tree.
+#else
+ size_type _M_node_count = 0; // Keeps track of size of tree.
+#endif
+#if __cplusplus < 201103L
_Rb_tree_impl()
- : _Node_allocator(), _M_key_compare(), _M_header(),
-_M_node_count(0)
- { _M_initialize(); }
+ : _M_node_count(0)
+ { }
+#else
+ _Rb_tree_impl() = default;
+#endif
The default constructor of the associative containers is required to
value-initialize the comparator (see their synopses in
[map/set/multimap/multiset.overview]).
I don't have latest Standard version so can't see the exact word but
I find quite annoying that the Standard doesn't allow this simple
implementation.
I don't know if unodered containers have same kind of requirements
for equal or hash functors but if so current implementation doesn't
do this value initialization.
So here is another attempt. This time it simply allows to have
noexcept condition in one place and closer to where operations are
being invoked.
Ok to commit after tests ?
François
_Rb_tree_impl() = default; doesn't do that; it default-initializes the
comparator instead.
Tim
diff --git a/libstdc++-v3/include/bits/stl_map.h
b/libstdc++-v3/include/bits/stl_map.h
index e5b2a1b..dea7d5b 100644
--- a/libstdc++-v3/include/bits/stl_map.h
+++ b/libstdc++-v3/include/bits/stl_map.h
@@ -167,11 +167,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
/**
* @brief Default constructor creates no elements.
*/
- map()
- _GLIBCXX_NOEXCEPT_IF(
- is_nothrow_default_constructible::value
- && is_nothrow_default_constructible::value)
- : _M_t() { }
+#if __cplusplus < 201103L
+ map() : _M_t() { }
+#else
+ map() = default;
+#endif
/**
* @brief Creates a %map with no elements.
diff --git a/libstdc++-v3/include/bits/stl_multimap.h
b/libstdc++-v3/include/bits/stl_multimap.h
index d240427..7e86b76 100644
--- a/libstdc++-v3/include/bits/stl_multimap.h
+++ b/libstdc++-v3/include/bits/stl_multimap.h
@@ -164,11 +164,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
/**
* @brief Default constructor creates no elements.
*/
- multimap()
- _GLIBCXX_NOEXCEPT_IF(
- is_nothrow_default_constructible::value
- && is_nothrow_default_constructible::value)
- : _M_t() { }
+#if __cplusplus < 201103L
+ multimap() : _M_t() { }
+#else
+ multimap() = default;
+#endif
/**
* @brief Creates a %multimap with no elements.
diff --git a/libstdc++-v3/include/bits/stl_multiset.h
b/libstdc++-v3/include/bits/stl_multiset.h
index cc068a9..7fe2fbd 100644
--- a/libstdc++-v3/include/bits/stl_multiset.h
+++ b/libstdc++-v3/include/bits/stl_multiset.h
@@ -144,11 +144,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
/**
* @brief Default constructor creates no elements.
*/
- multiset()
- _GLIBCXX_NOEXCEPT_IF(
- is_nothrow_default_constructible::value
- && is_nothrow_default_constructible::value)
- : _M_t() { }
+#if __cplusplus < 201103L
+ multiset() : _M_t() { }
+#else
+ multiset() = default;
+#endif
/**
* @brief Creates a %multiset with no elements.
diff --git a/libstdc++-v3/include/bits/stl_set.h
b/libstdc++-v3/include/bits/stl_set.h
index 3938351..5ed9672 100644
--- a/libstdc++-v3/include/bits/stl_set.h
+++ b/libstdc++-v3/include/bits/stl_set.h
@@ -147,11 +147,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
/**
* @brief Default constructor creates no elements.
*/
- set()
- _GLIBCXX_NOEXCEPT_IF(
- is_nothrow_default_constructible::value
- && is_nothrow_default_constructible::value)
- : _M_t() { }
+#if __cplusplus < 201103L
+ set() : _M_t() { }
+#else
+ set() = default;
+#endif
/**
* @brief Creates a %set with no elements.
diff --git a/libstdc++-v3/include/bits/stl_tree.h
b/libstdc++-v3/include/bits/stl_tree.h
index ee2dc70..b6a3c1e 100644
--- a/libstdc++-v3/include/bits/stl_tree.h
+++ b/libstdc++-v3/include/bits/stl_tree.h
@@ -137,6 +137,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
}
};
+ struct _Rb_header_node : public _Rb_tree_node_base
+ {
+_Rb_header_node() _GLIBCXX_NOEXCEPT
+{
+ _M_color = _S_red;
+ _M_parent = _Base_ptr();
+ _M_left = _M_right = this;
+}
+ };
+
template
struct _Rb_tree_node : public _Rb_tree_node_base
{
@@ -602,24 +612,34 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
struct _Rb_tree_impl : public _Node_allocator
{
_Key_compare_M_key_compare
[PATCH] Fix dwarf2out regression from C++17 inline variables changes
On Tue, Oct 25, 2016 at 08:06:12PM +0200, Jakub Jelinek wrote:
> I think this patch should fix it, will bootstrap/regtest it now:
>
> 2016-10-25 Jakub Jelinek
>
> * dwarf2out.c (gen_member_die): Only reparent_child instead of
> splice_child_die if child doesn't have DW_AT_specification attribute.
Bootstrapped/regtested on x86_64-linux and i686-linux now, with this
patch C++14 static data members are treated in debug info as they were in
the past, just C++17 static constexpr data mambers or other static inline
data members are treated the new way (definition in DW_TAG_member).
Ok for trunk?
> --- gcc/dwarf2out.c.jj2016-10-25 19:49:28.0 +0200
> +++ gcc/dwarf2out.c 2016-10-25 20:02:33.264639847 +0200
> @@ -22624,7 +22624,8 @@ gen_member_die (tree type, dw_die_ref co
> /* Handle inline static data members, which only have in-class
>declarations. */
> if (child->die_tag == DW_TAG_variable
> - && child->die_parent == comp_unit_die ())
> + && child->die_parent == comp_unit_die ()
> + && get_AT (child, DW_AT_specification) == NULL)
> {
> reparent_child (child, context_die);
> child->die_tag = DW_TAG_member;
>
Jakub
Re: [PATCH] Fix not caught use-after-scope with -O1 (PR sanitize/78106)
On Tue, Oct 25, 2016 at 02:16:33PM +0200, Martin Liška wrote:
> 2016-10-25 Martin Liska
>
> PR sanitizer/78106
> * sanopt.c (imm_dom_path_with_freeing_call): Handle gasm
> statements as they can also contain possibly a freeing call.
>
> gcc/testsuite/ChangeLog:
>
> 2016-10-25 Martin Liska
>
> PR sanitizer/78106
> * gcc.dg/asan/pr78106.c: New test.
The test is UNRESOLVED with -flto, fixed thusly, regtested on x86_64-linux,
committed to trunk as obvious:
2016-10-25 Jakub Jelinek
PR sanitizer/78106
* gcc.dg/asan/pr78106.c: Add -ffat-lto-objects to dg-options.
--- gcc/testsuite/gcc.dg/asan/pr78106.c.jj 2016-10-25 20:06:45.0
+0200
+++ gcc/testsuite/gcc.dg/asan/pr78106.c 2016-10-25 22:08:09.712879270 +0200
@@ -1,6 +1,6 @@
/* PR sanitizer/78106 */
/* { dg-do compile } */
-/* { dg-options "-fsanitize=address -fdump-tree-sanopt-details" } */
+/* { dg-options "-fsanitize=address -fdump-tree-sanopt-details
-ffat-lto-objects" } */
int *variable;
Jakub
[SPARC] Housekeeping work
No functional changes.
Tested on SPARC/Solaris, applied on the mainline.
2016-10-25 Eric Botcazou
* config.gcc (sparc*-*-solaris2*): Adjust.
(sparc64-*-linux*): Likewise.
* config/sparc/default-64.h: Rename to...
* config/sparc/default64.h: ...this.
* config/sparc/sparc.c (sparc_option_override): Replace TARGET_64BIT
with TARGET_ARCH64.
(sparc_mangle_type): Replace !TARGET_64BIT with TARGET_ARCH32.
* config/sparc/sparc.h: Minor tweaks.
* config/sparc/sparc.md: Replace !TARGET_64BIT and !TARGET_ARCH64 with
TARGET_ARCH32 throughout. Minor various tweaks throughout.
--
Eric BotcazouIndex: config.gcc
===
--- config.gcc (revision 241437)
+++ config.gcc (working copy)
@@ -2783,7 +2783,7 @@ sparc*-*-solaris2*)
tm_file="sparc/biarch64.h ${tm_file} ${sol2_tm_file} sparc/tso.h"
case ${target} in
sparc64-*-* | sparcv9-*-*)
- tm_file="sparc/default-64.h ${tm_file}"
+ tm_file="sparc/default64.h ${tm_file}"
;;
*)
test x$with_cpu != x || with_cpu=v9
@@ -2806,7 +2806,7 @@ sparc64-*-rtems*)
tmake_file="${tmake_file} sparc/t-sparc sparc/t-rtems-64"
;;
sparc64-*-linux*)
- tm_file="sparc/biarch64.h ${tm_file} dbxelf.h elfos.h sparc/sysv4.h gnu-user.h linux.h glibc-stdint.h sparc/default-64.h sparc/linux64.h sparc/tso.h"
+ tm_file="sparc/biarch64.h ${tm_file} dbxelf.h elfos.h sparc/sysv4.h gnu-user.h linux.h glibc-stdint.h sparc/default64.h sparc/linux64.h sparc/tso.h"
extra_options="${extra_options} sparc/long-double-switch.opt"
tmake_file="${tmake_file} sparc/t-sparc sparc/t-linux64"
;;
Index: config/sparc/default-64.h
===
--- config/sparc/default-64.h (revision 241437)
+++ config/sparc/default-64.h (working copy)
@@ -1,22 +0,0 @@
-/* Definitions of target machine for GCC, for bi-arch SPARC,
- defaulting to 64-bit code generation.
-
- Copyright (C) 1999-2016 Free Software Foundation, Inc.
-
-This file is part of GCC.
-
-GCC is free software; you can redistribute it and/or modify
-it under the terms of the GNU General Public License as published by
-the Free Software Foundation; either version 3, or (at your option)
-any later version.
-
-GCC is distributed in the hope that it will be useful,
-but WITHOUT ANY WARRANTY; without even the implied warranty of
-MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
-GNU General Public License for more details.
-
-You should have received a copy of the GNU General Public License
-along with GCC; see the file COPYING3. If not see
-
Re: [patch, fortran] PR45516 - [F08] allocatable components of recursive type
Committed as revision 241539.
Thanks for all the help, Andre and Dominique.
Paul
On 25 October 2016 at 16:09, Andre Vehreschild wrote:
> Hi Paul,
>
> Just one small nit and one idea remaining:
>
> The nit: You've missed one "same_type" at:
>
>> Index: gcc/fortran/trans-array.c
>> ===
>> *** gcc/fortran/trans-array.c (revision 241467)
>> --- gcc/fortran/trans-array.c (working copy)
>> *** structure_alloc_comps (gfc_symbol * der_
>> *** 8421,8426
>> --- 8510,8516
>> gfc_add_expr_to_block (&fnblock, tmp);
>> }
>> else if (c->attr.allocatable && !c->attr.proc_pointer
>> + && !(c->ts.type == BT_DERIVED && der_type ==
>> c->ts.u.derived) && (!(cmp_has_alloc_comps && c->as)
>
> here
>>|| c->attr.codimension))
>> {
>
> The idea: One could think about doing the same "same_type" for the chunks in
> ===
> *** gcc/fortran/trans-types.c (revision 241467)
> --- gcc/fortran/trans-types.c (working copy)
> *** gfc_get_derived_type (gfc_symbol * deriv
>
> which would eliminate evaluating the same checks thrice.
>
> Besides that ok for trunk.
>
> Thanks for the work.
>
> Regards,
> Andre
>
>
> On Tue, 25 Oct 2016 13:40:03 +0200
> Paul Richard Thomas wrote:
>
>> Dear Andre,
>>
>> Thanks for the review. Please find my responses below and the revised
>> patch attached
>>
>> With these changes, OK for trunk?
>>
>> With respect to Dominique's testcase posted last night at 23:18, this
>> is an issue for the feature which I will raise on clf. It seems to me
>> that in the circumstances, a deep copy of the allocatable components
>> should be made in the FROM expression in move_alloc but I am not sure.
>> I will post a PR about this as well, after the patch is committed.
>>
>> Best regards
>>
>> Paul
>>
>> snip
>>
>> >
>> > So recursive types are defined to be simple recursive, i.e., not
>> > transitive?
>> > type A == uses => type B == uses => type A
>>
>> This is caught by:
>> type(linked_list2), allocatable :: link
>>1
>> Error: Derived type at (1) has not been previously defined and so
>> cannot appear in a derived type definition
>> andre.f90:10:28:
>>
>>type(linked_list1) :: test
>> 1
>> Error: Symbol ‘test’ at (1) cannot have a type
>> andre.f90:12:27:
>>
>> So, in this implementation anyway, the answer to your question is 'yes'.
>>
>> snip
>>
>> >> + if (derived->attr.unlimited_polymorphic
>> >> + || derived->attr.abstract
>> >> + || !rdt)
>> >
>> > When unlimited or abstract rdt can never be true. This is redundant here,
>> > isn't it?
>>
>> No. Derived types that are not unlimited or abstract might not have
>> recursive components.
>>
>> snip
>>
>> >> {
>> >> if (!c->attr.pointer && !c->attr.proc_pointer
>> >> + && !(c->attr.allocatable && (der == c->ts.u.derived))
>> >
>> > The inner most pair of parentheses is unnecessary here.
>>
>> The parentheses have been removed.
>>
>> snip
>>
>> > The indentation of the whole block above is wrong. The comment has 6 spaces
>> > while it should have 2 only.
>>
>> Corrected. Well spotted.
>>
>> snip
>>
>> > is_derived_type = c->ts.type == BT_DERIVED && der_type == c->ts.u.derived;
>> >
>> > and using that instead of repeatedly evaluating the expression?
>>
>> Done. I called it 'same_type'.
>> >
>> >
>> >
>> >> *** structure_alloc_comps (gfc_symbol * der_
>> >> *** 8137,8142
>> >> --- 8141,8222
>> >> build_int_cst (TREE_TYPE (comp), 0));
>> >> gfc_add_expr_to_block (&tmpblock, tmp);
>> >> }
>> >> + else if (c->attr.allocatable && !c->attr.codimension)
>> >> + {
>> >
>> > How about putting also the creation of the descriptor into the body that is
>> > guarded by the is_allocated, i.e. in pseudo-code make:
>>
>> snip
>>
>> done
>>
>> >
>> > Furthermore, why are you always using an array? We do now at
>> > code-generation
>> > time whether the argument is a scalar or an array, don't we? This is more
>> > for my understanding, then an actual comment.
>>
>> This is to limit the number of deallocators to 1 with a rank=1
>> argument. This was the simplest solution.
>>
>> >
>> >> /* The size field is returned as an array index type. Therefore treat
>> >> Index: gcc/fortran/trans-types.c
>> >> ===
>> >> *** gcc/fortran/trans-types.c (revision 241467)
>> >> --- gcc/fortran/trans-types.c (working copy)
>>
>> snip
>>
>> >> ! && !(c->attr.allocatable && (derived == c->ts.u.derived))
>> >
>> > Spare parentheses above (derived == ..)
>>
>> All remov
Re: [PATCH, Fortran] DEC Compatibility: New I/O Specifiers CARRIAGECONTROL, READONLY, SHARE with -fdec
On 10/25/2016 11:52 AM, Fritz Reese wrote: All, Here's the big one. This patch proposes an extension to both the GNU Fortran front-end and runtime library (libgfortran) to support three additional I/O specifiers: CARRIAGECONTROL, READONLY, and SHARE for compatibility with legacy compilers/code. Here's a summary of what the specifiers actually do: CARRIAGECONTROL is to control line termination settings between output records. READONLY implies ACTION='READ' and additionally prevents STATUS='DELETE' from deleting a file on CLOSE. SHARE provides OS-level locks. These specifiers are designed to match the syntax and behavior of legacy compilers. For more info see the attached test cases, source code, and documentation. Each comes with an IOPARM_OPEN_ bit, and CC+SHARE have IOPARM_INQUIRE_ bits. The st_parameter_dt structure needs an additional two bytes to propagate information with CARRIAGECONTROL='FORTRAN'. These two bytes still leave plenty of trailing 'pad' for future expansion. Bootstraps and regtests on x86_64-redhat-linux. There's a fair bit to sift through, so feel free to ask for clarification or provide comments/constructive criticism. OK for trunk? I have started reviewing this. I assume you have tested on some if not all of your DEC code base. Have you compared results with ifort? How have you tested the SHARE feature? Do you have a test with multiple threads accessing a single file? or do I misunderstand that feature? (I don't mean in the testsuite, I mean as just your own testing.) Jerry
MAINTAINERS update
Hi, Adding myself to MAINTAINERS (Write After Approval). (r241541) Thanks, -Will Index: ChangeLog === --- ChangeLog (revision 241538) +++ ChangeLog (working copy) @@ -1,3 +1,7 @@ +2016-10-25 Will Schmidt + + * MAINTAINERS (Write After Approval): Add myself. + 2016-10-21 Hartmut Penner Ulrich Weigand Index: MAINTAINERS === --- MAINTAINERS (revision 241538) +++ MAINTAINERS (working copy) @@ -554,6 +554,7 @@ Trevor Saunders Aaron Sawdey Roger Sayle +Will Schmidt William Schmidt Tilo Schwarz Martin Sebor
Re: [PATCH/AARCH64] Handle ILP32 multi-arch
On 07.10.2016 23:08, Andrew Pinski wrote: > Hi, > This patch adds ilp32 multi-arch support. This is needed to support > multi-arch on Debian like systems. > > OK? Bootstrapped and tested on aarch64-linux-gnu with no regressions. > Also tested with ilp32 with a newly built toolchain that supports > ILP32 with Ubuntu 1604 base. > > Thanks, > Andrew > > ChangeLog: > * config/aarch64/t-aarch64-linux (MULTILIB_OSDIRNAMES): Handle > multi-arch for ilp32. I can't approve that, but it looks like a reasonable change, but we should document the multiarch triplet at https://wiki.debian.org/Multiarch/Tuples Matthias
Make vax backend -Wfallthru compliant
This adds /* FALLTHRU */ comments to the vax backend. It's the backend
changes necessary to build with the trunk. There's also a change for
lra-constraints, but I'll send that separately.
Applied to the trunk.
Jeff
commit 1af18c59cb158024a77068b99875cd7db9d8b8bf
Author: law
Date: Tue Oct 25 22:31:57 2016 +
* config/vax/vax.c (vad_address_cost_1): Add missing FALLTHRU comment.
(vax_notice_update_cc): Likewise.
git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@241542
138bc75d-0d04-0410-961f-82ee72b054a4
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index b442d54..465761f 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,8 @@
+2016-10-25 Jeff Law
+
+ * config/vax/vax.c (vad_address_cost_1): Add missing FALLTHRU comment.
+ (vax_notice_update_cc): Likewise.
+
2016-10-25 Eric Botcazou
* config.gcc (sparc*-*-solaris2*): Adjust.
diff --git a/gcc/config/vax/vax.c b/gcc/config/vax/vax.c
index 732ebee..60793268 100644
--- a/gcc/config/vax/vax.c
+++ b/gcc/config/vax/vax.c
@@ -678,6 +678,7 @@ vax_address_cost_1 (rtx addr)
{
case PRE_DEC:
predec = 1;
+ /* FALLTHRU */
case REG:
case SUBREG:
case POST_INC:
@@ -1084,6 +1085,7 @@ vax_notice_update_cc (rtx exp, rtx insn ATTRIBUTE_UNUSED)
case NEG:
if (GET_MODE_CLASS (GET_MODE (exp)) == MODE_FLOAT)
break;
+ /* FALLTHRU */
case AND:
case IOR:
case XOR:
Re: [PATCH, Fortran] DEC Compatibility: New I/O Specifiers CARRIAGECONTROL, READONLY, SHARE with -fdec
On Tue, Oct 25, 2016 at 5:29 PM, Jerry DeLisle wrote: > On 10/25/2016 11:52 AM, Fritz Reese wrote: >> >> All, >> >> Here's the big one. This patch proposes an extension to both the GNU >> Fortran front-end and runtime library (libgfortran) to support three >> additional I/O specifiers: CARRIAGECONTROL, READONLY, and SHARE for >> compatibility with legacy compilers/code. ... >> > I have started reviewing this. I assume you have tested on some if not all > of your DEC code base. Have you compared results with ifort? Yes, my original development process is to match outputs with the version of ifort I have where it makes sense. I have compared the CARRIAGECONTROL in particular quite extensively with ifort and it gives the same output for inputs I can think of, which mostly comprise of the ones in the DG tests. READONLY functions the same. > How have you tested the SHARE feature? Do you have a test with multiple > threads accessing a single file? or do I misunderstand that feature? (I > don't mean in the testsuite, I mean as just your own testing.) For SHARE, I had test programs open files with the various SHARE= types and try to read/write them from separate processes (manually through separate terminal windows), verifying that the locking occurs according to the OS specs. I believe you understand correctly. I also used 'strace' on those programs to verify the OS calls occured in the correct order. Not sure how I could put such tests into DG so I didn't bother. Thanks again for all the help with the reviews. --- Fritz Reese
[PATCH] PR78056: Fix build failure on Power7
This patch corrects an error introduced with commit 241314. That patch
introduced several new built-in functions to support Power9 string
instructions. The error that was found subsequent to the trunk commit
is that initialization of the built-in function tables encounters an
internal compiler error if the assembler that is used with gcc lacks
support for Power9 instructions.
This patch disables initialization of built-in functions which depend
on assembler capabilities that are not supported by the associated tool
chain.
This patch has been booted and regression tested on
powerpcle-unknown-linux, trunk revision 241406. (I was not able to
regression test on the most current trunk because that trunk does not
boot.) I have also successfully boot-strapped this patch on a Power7
system for which the assembler lacks support for Power9. (I could not
regression test on that platform because that platform could not
bootstrap without this patch.)
It is planned that a subsequent enhancement to this patch will make the
following improvements:
1. Fail with an assertion error instead of an internal compiler error
if built-in functions are ever defined for which the corresponding
instruction pattern is not supported by the current compiler
configuration.
2. Issue a warning message whenever a command-line -mcpu=XXX request
seeks to configure support for a CPU version which is not supported by
the accompanying assembler.
I am submitting the patch as is in order to expedite integration since
the error has broken the trunk for certain system configurations.
Is this patch ok for trunk?
gcc/ChangeLog:
2016-10-25 Kelvin Nilsen
PR target/78056
* config/rs6000/rs6000.c (spe_init_builtins): Modify loops to not
define builtin functions from the bdesc_spe_predicates or
bdesc_spe_evsel arrays if the builtin mask is not compatible with
the current compiler configuration.
(paired_init_builtins): Modify loop to not define define builtin
functions from the bdesc_paried_preds array if the builtin mask is
not compatible with the current compiler configuration.
(altivec_init_builtins): Modify loops to not define the
__builtin_altivec_stxvl function nor the builtin functions from
the bdesc_dst or bdesc_altivec_preds, bdesc_abs
gcc/testsuite/ChangeLog:
2016-10-25 Kelvin Nilsen
PR target/78056
* gcc.target/powerpc/vsu/vec-any-eqz-7.c (test_any_equal): Change
expected error message.
* gcc.target/powerpc/vsu/vec-xst-len-12.c (store_data): Change
expected error message.
* gcc.target/powerpc/vsu/vec-all-nez-7.c
(test_all_not_equal_and_not_zero): Change expected error message.
Index: gcc/config/rs6000/rs6000.c
===
--- gcc/config/rs6000/rs6000.c (revision 241406)
+++ gcc/config/rs6000/rs6000.c (working copy)
@@ -16923,6 +16923,7 @@ spe_init_builtins (void)
tree pushort_type_node = build_pointer_type (short_unsigned_type_node);
const struct builtin_description *d;
size_t i;
+ HOST_WIDE_INT builtin_mask = rs6000_builtin_mask;
tree v2si_ftype_4_v2si
= build_function_type_list (opaque_V2SI_type_node,
@@ -17063,7 +17064,16 @@ spe_init_builtins (void)
for (i = 0; i < ARRAY_SIZE (bdesc_spe_predicates); ++i, d++)
{
tree type;
+ HOST_WIDE_INT mask = d->mask;
+ if ((mask & builtin_mask) != mask)
+ {
+ if (TARGET_DEBUG_BUILTIN)
+ fprintf (stderr, "spe_init_builtins, skip predicate %s\n",
+d->name);
+ continue;
+ }
+
switch (insn_data[d->icode].operand[1].mode)
{
case V2SImode:
@@ -17084,7 +17094,16 @@ spe_init_builtins (void)
for (i = 0; i < ARRAY_SIZE (bdesc_spe_evsel); ++i, d++)
{
tree type;
+ HOST_WIDE_INT mask = d->mask;
+ if ((mask & builtin_mask) != mask)
+ {
+ if (TARGET_DEBUG_BUILTIN)
+ fprintf (stderr, "spe_init_builtins, skip evsel %s\n",
+d->name);
+ continue;
+ }
+
switch (insn_data[d->icode].operand[1].mode)
{
case V2SImode:
@@ -17106,6 +17125,7 @@ paired_init_builtins (void)
{
const struct builtin_description *d;
size_t i;
+ HOST_WIDE_INT builtin_mask = rs6000_builtin_mask;
tree int_ftype_int_v2sf_v2sf
= build_function_type_list (integer_type_node,
@@ -17141,7 +17161,16 @@ paired_init_builtins (void)
for (i = 0; i < ARRAY_SIZE (bdesc_paired_preds); ++i, d++)
{
tree type;
+ HOST_WIDE_INT mask = d->mask;
+ if ((mask & builtin_mask) != mask)
+ {
+ if (TARGET_DEBUG_BUILTIN)
+ fprintf (stderr, "paired_init_builtins, skip predicate %s\n",
+d->name);
+ continue;
+ }
+
if (TARGET_DEBUG_BUILTIN)
fprintf (stderr, "paired pred #%d, insn = %s [%d], mode = %s\n",
Re: [PATCH, Fortran] DEC Compatibility: New I/O Specifiers CARRIAGECONTROL, READONLY, SHARE with -fdec
On 10/25/2016 11:52 AM, Fritz Reese wrote: All, Here's the big one. This patch proposes an extension to both the GNU Fortran front-end and runtime library (libgfortran) to support three additional I/O specifiers: CARRIAGECONTROL, READONLY, and SHARE for compatibility with legacy compilers/code. Here's a summary of what the specifiers actually do: CARRIAGECONTROL is to control line termination settings between output records. READONLY implies ACTION='READ' and additionally prevents STATUS='DELETE' from deleting a file on CLOSE. SHARE provides OS-level locks. These specifiers are designed to match the syntax and behavior of legacy compilers. For more info see the attached test cases, source code, and documentation. Each comes with an IOPARM_OPEN_ bit, and CC+SHARE have IOPARM_INQUIRE_ bits. The st_parameter_dt structure needs an additional two bytes to propagate information with CARRIAGECONTROL='FORTRAN'. These two bytes still leave plenty of trailing 'pad' for future expansion. Bootstraps and regtests on x86_64-redhat-linux. There's a fair bit to sift through, so feel free to ask for clarification or provide comments/constructive criticism. OK for trunk? Patch applies with a few minor offsets, regression tests OK, indentation looks good. Test cases are thorough. I appreciate that you guarded the open_share and close_share with the #if defined(HAVE_FCNTL) && defined(F_SETLK) && defined(F_UNLCK). I am pretty sure not all platforms will support these. Good Job, Yes, OK to commit. Jerry
