[PATCH, middle-end/i386]: Fix PR88502, Inline built-in asinh, acosh, atanh for -ffast-math
Attached patch inlines calls to asinh{,f}, acosh{,f,l} and atanh{,f,l}
using x87 XFmode arithmetic. In the patch, I left out asinhl due to
its reduced input argument range, but perhaps it could be added back,
since we are expanding under flag_unsafe_math_optimizations. The
expanders are modelled after the removed inlines in glibc [1] (which
also include asinhl, with a comment mentioning its reduced input
argument range).
2018-12-17 Uros Bizjak
PR target/88502
* internal-fn.def (ACOSH): New.
(ASINH): Ditto.
(ATANH): Ditto.
* optabs.def (acosh_optab): New.
(asinh_optab): Ditto.
(atanh_optab): Ditto.
* config/i386/i386-protos.h (ix86_emit_i387_asinh): New prototype.
(ix86_emit_i387_acosh): Ditto.
(ix86_emit_i387_atanh): Ditto.
* config/i386/i386.c (ix86_emit_i387_asinh): New function.
(ix86_emit_i387_acosh): Ditto.
(ix86_emit_i387_atanh): Ditto.
* config/i386/i386.md (asinh2): New expander.
(acoshxf2): Ditto.
(acosh2): Ditto.
(atanhxf2): Ditto.
(atanh2): Ditto.
Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.
The patch also needs approval for its straightforward middle-end parts.
[1] https://sourceware.org/ml/libc-alpha/2018-12/msg00519.html
Uros.
Re: [PATCH] x86: Add -march=cascadelake
On Sun, Dec 16, 2018 at 4:15 PM Wei Xiao wrote:
>
> Thanks for the comments!
> Fixed as attached.
> Ok for trunk?
OK.
Thanks,
Uros.
> Jakub Jelinek 于2018年12月14日周五 下午6:47写道：
> >
> > On Fri, Dec 14, 2018 at 06:33:37PM +0800, Wei Xiao wrote:
> > --- a/gcc/config/i386/driver-i386.c
> > +++ b/gcc/config/i386/driver-i386.c
> > @@ -832,8 +832,16 @@ const char *host_detect_local_cpu (int argc, const
> > char **argv)
> > cpu = "skylake";
> > break;
> > case 0x55:
> > - /* Skylake with AVX-512. */
> > - cpu = "skylake-avx512";
> > + if (has_avx512vnni)
> > + {
> > + /* Cascade Lake. */
> > + cpu = "cascadelake";
> > + }
> > + else
> > + {
> > + /* Skylake with AVX-512. */
> > + cpu = "skylake-avx512";
> > + }
> > break;
> >
> > Just a formatting nit here, if {}s are used, they should be indented
> > 2 columns to the right from the if or else and the body of {} should
> > be indented by two further columns over {.
> > But, in this case, there is another rule, that if the body has a single
> > statement, then there shouldn't be {}s around it. Thus just:
> > if (has_avx512vnni)
> > /* Cascade Lake. */
> > cpu = "cascadelake";
> > else
> > /* Skylake with AVX-512. */
> > cpu = "skylake-avx512";
> >
> > Jakub
[PATCH, og8] Add OpenACC 2.6 `serial' construct support
The `serial' construct is equivalent to a `parallel' construct with clauses `num_gangs(1) num_workers(1) vector_length(1)' implied. Naturally these clauses are therefore not supported with the `serial' construct. All the remaining clauses accepted with `parallel' are also accepted with `serial'. Consequently implementation is straightforward, by handling `serial' exactly like `parallel', except for hardcoding dimensions rather than taking them from the relevant clauses, in `expand_omp_target'. Separate codes are used to denote the `serial' construct throughout the middle end, even though the mapping of `serial' to an equivalent `parallel' construct could have been done in the individual language frontends, saving a lot of mechanical changes and avoiding middle-end code expansion. This is so that any reporting such as with warning or error messages and in diagnostic dumps use `serial' rather than `parallel', therefore avoiding user confusion. gcc/ * gimple.h (gf_mask): Add GF_OMP_TARGET_KIND_OACC_SERIAL enumeration constant. (is_gimple_omp_oacc): Handle GF_OMP_TARGET_KIND_OACC_SERIAL. (is_gimple_omp_offloaded): Likewise. * gimplify.c (omp_region_type): Add ORT_ACC_SERIAL enumeration constant. Adjust the value of ORT_NONE accordingly. (is_gimple_stmt): Handle OACC_SERIAL. (omp_add_variable): Handle ORT_ACC_SERIAL. (oacc_default_clause): Likewise. (gimplify_scan_omp_clauses): Likewise. (gomp_needs_data_present): Likewise. (gimplify_adjust_omp_clauses): Likewise. (gimplify_omp_workshare): Handle OACC_SERIAL. (gimplify_expr): Likewise. * omp-expand.c (expand_omp_target): Handle GF_OMP_TARGET_KIND_OACC_SERIAL. (build_omp_regions_1, omp_make_gimple_edges): Likewise. * omp-low.c (is_oacc_parallel): Rename function to... (is_oacc_parallel_or_serial): ... this. Handle GF_OMP_TARGET_KIND_OACC_SERIAL. (build_receiver_ref): Adjust accordingly. (build_sender_ref): Likewise. (scan_sharing_clauses): Likewise. (create_omp_child_function): Likewise. (scan_omp_for): Likewise. (scan_omp_target): Likewise. (lower_oacc_head_mark): Likewise. (convert_from_firstprivate_int): Likewise. (lower_omp_target): Likewise. (check_omp_nesting_restrictions): Handle GF_OMP_TARGET_KIND_OACC_SERIAL. (lower_oacc_reductions): Likewise. (lower_omp_target): Likewise. * tree-pretty-print.c (dump_generic_node): Handle OACC_SERIAL. * tree.def (OACC_SERIAL): New tree code. * doc/generic.texi (OpenACC): Document OACC_SERIAL. gcc/c-family/ * c-pragma.h (pragma_kind): Add PRAGMA_OACC_SERIAL enumeration constant. * c-pragma.c (oacc_pragmas): Add "serial" entry. gcc/c/ * c-parser.c (OACC_SERIAL_CLAUSE_MASK): New macro. (OACC_SERIAL_CLAUSE_DEVICE_TYPE_MASK): Likewise. (c_parser_oacc_kernels_parallel): Rename function to... (c_parser_oacc_compute): ... this. Handle PRAGMA_OACC_SERIAL. (c_parser_omp_construct): Update accordingly. gcc/cp/ * constexpr.c (potential_constant_expression_1): Handle OACC_SERIAL. * parser.c (OACC_SERIAL_CLAUSE_MASK): New macro. (OACC_SERIAL_CLAUSE_DEVICE_TYPE_MASK): Likewise. (cp_parser_oacc_kernels_parallel): Rename function to... (cp_parser_oacc_compute): ... this. Handle PRAGMA_OACC_SERIAL. (cp_parser_omp_construct): Update accordingly. (cp_parser_pragma): Handle PRAGMA_OACC_SERIAL. Fix alphabetic order. * pt.c (tsubst_expr): Handle OACC_SERIAL. gcc/fortran/ * gfortran.h (gfc_statement): Add ST_OACC_SERIAL_LOOP, ST_OACC_END_SERIAL_LOOP, ST_OACC_SERIAL and ST_OACC_END_SERIAL enumeration constants. (gfc_exec_op): Add EXEC_OACC_SERIAL_LOOP and EXEC_OACC_SERIAL enumeration constants. * match.h (gfc_match_oacc_serial): New prototype. (gfc_match_oacc_serial_loop): Likewise. * dump-parse-tree.c (show_omp_node, show_code_node): Handle EXEC_OACC_SERIAL_LOOP and EXEC_OACC_SERIAL. * match.c (match_exit_cycle): Handle EXEC_OACC_SERIAL_LOOP. * openmp.c (OACC_SERIAL_CLAUSES): New macro. (OACC_SERIAL_CLAUSE_DEVICE_TYPE_MASK): Likewise. (gfc_match_oacc_serial_loop): New function. (gfc_match_oacc_serial): Likewise. (oacc_is_loop): Handle EXEC_OACC_SERIAL_LOOP. (resolve_omp_clauses): Handle EXEC_OACC_SERIAL. (oacc_is_serial): New function. (oacc_code_to_statement): Handle EXEC_OACC_SERIAL and EXEC_OACC_SERIAL_LOOP. (gfc_resolve_oacc_directive): Likewise. (resolve_oacc_loop_blocks): Also call `oacc_is_serial'. * parse.c (decode_oacc_directive) <'s'>: Add case for "serial"
[Committed] PR fortran/85314 -- Remove dead code.
Committed as obvous.
2018-12-16 Steven G. Kargl
* resolve.c (resolve_transfer): Remove dead code.
Index: gcc/fortran/resolve.c
===
--- gcc/fortran/resolve.c (revision 267190)
+++ gcc/fortran/resolve.c (working copy)
@@ -9386,10 +9386,7 @@ resolve_transfer (gfc_code *code)
if (dt && dt->dt_io_kind->value.iokind != M_INQUIRE
&& (ts->type == BT_DERIVED || ts->type == BT_CLASS))
{
- if (ts->type == BT_DERIVED || ts->type == BT_CLASS)
- derived = ts->u.derived;
- else
- derived = ts->u.derived->components->ts.u.derived;
+ derived = ts->u.derived;
/* Determine when to use the formatted DTIO procedure. */
if (dt && (dt->format_expr || dt->format_label))
--
Steve
Re: [patch] Fix bootstrap powerpc*-*-freebsd* targets
On Sun, Dec 16, 2018 at 07:46:18PM +0100, Andreas Tobler wrote:
> On 12.12.18 21:32, Andreas Tobler wrote:
>
> > this patch fixes bootstrap for my powerpc*-unknown-freebsd* targets.
> > The definition of GNU_USER_DYNAMIC_LINKER was recently moved to linux.h.
Sorry about the breakage.
> > But the GNU_USER_DYNAMIC_LINKER is still used in rs6000/sysv4.h.
> > So I add an empty definition with guard to cure the bootstrap issue.
> >
> > Ok for trunk?
>
> This is another attemtp to fix bootstrap on powerpc*-unknown-freebsd* only.
>
> This one is only for FreeBSD and so I can approve and apply myself.
>
> Objections?
It's not the best fix. An empty string for something that is an
argument of -dynamic-linker will result in the linker eating the next
command line string for -dynamic-linker. That will break -mcall-linux
on freebsd, if anyone should try to use that option. (Chances are
that most of the -mcall options are broken anyway, but let's not break
them further.)
Since I broke powerpc*-freebsd and the other non-linux powerpc
targets, I guess I ought to fix them. The following is a variation on
your first patch, that results in -mcall-linux for powerpc-freebsd*
providing the 32-bit powerpc-linux dynamic linker.
Tested with x86_64-linux to powerpc-linux, powerpc64le-linux and
powerpc-freebsd12.0 crosses, observing the effect of -muclibc, -mmusl
etc. on powerpc*-linux and -mcall-linux on powerpc-freebsd. I'll also
do a full native powerpc64le-linux bootstrap and regression test.
* config/rs6000/sysv4.h (GNU_USER_DYNAMIC_LINKER): Define.
diff --git a/gcc/config/rs6000/sysv4.h b/gcc/config/rs6000/sysv4.h
index 0d84f4f3f57..598da25269e 100644
--- a/gcc/config/rs6000/sysv4.h
+++ b/gcc/config/rs6000/sysv4.h
@@ -761,6 +761,10 @@ GNU_USER_TARGET_CC1_SPEC
#define MUSL_DYNAMIC_LINKER \
"/lib/ld-musl-powerpc" MUSL_DYNAMIC_LINKER_E "%{msoft-float:-sf}.so.1"
+#ifndef GNU_USER_DYNAMIC_LINKER
+#define GNU_USER_DYNAMIC_LINKER GLIBC_DYNAMIC_LINKER
+#endif
+
#define LINK_OS_LINUX_SPEC "-m elf32ppclinux %{!shared: %{!static: \
%{rdynamic:-export-dynamic} \
-dynamic-linker " GNU_USER_DYNAMIC_LINKER "}}"
--
Alan Modra
Australia Development Lab, IBM
[PATCH, libphobos] Committed commonize core.sys.posix.sys.msg
Hi,
This patch is a continuation of prior merging common version blocks to
help simplify porting the C bindings. For this particular module, it
now more closely resembles how glibc arranges msq.h, fixing a couple
of targets in the process, notably X32.
Bootstrapped and tested on x86_64-linux-gnu.
Committed to trunk as r267192.
--
Iain
---
diff --git a/libphobos/libdruntime/core/sys/posix/sys/msg.d b/libphobos/libdruntime/core/sys/posix/sys/msg.d
index 637df206cce..1957debbda7 100644
--- a/libphobos/libdruntime/core/sys/posix/sys/msg.d
+++ b/libphobos/libdruntime/core/sys/posix/sys/msg.d
@@ -6,421 +6,200 @@
module core.sys.posix.sys.msg;
+import core.sys.posix.config;
import core.sys.posix.sys.ipc;
-public import core.sys.posix.sys.types;
-public import core.stdc.config;
+import core.sys.posix.sys.types;
+import core.stdc.config;
version (CRuntime_Glibc):
// Some of these may be from linux kernel headers.
extern (C):
-public enum MSG_STAT = 11;
-public enum MSG_INFO = 12;
+version (ARM) version = ARM_Any;
+version (AArch64) version = ARM_Any;
+version (HPPA)version = HPPA_Any;
+version (HPPA64) version = HPPA_Any;
+version (MIPS32) version = MIPS_Any;
+version (MIPS64) version = MIPS_Any;
+version (PPC) version = PPC_Any;
+version (PPC64) version = PPC_Any;
+version (RISCV32) version = RISCV_Any;
+version (RISCV64) version = RISCV_Any;
+version (S390)version = IBMZ_Any;
+version (SPARC) version = SPARC_Any;
+version (SPARC64) version = SPARC_Any;
+version (SystemZ) version = IBMZ_Any;
+
+version (linux)
+{
+enum MSG_STAT = 11;
+enum MSG_INFO = 12;
+
+enum MSG_NOERROR = 1 << 12; // octal!1
+enum MSG_EXCEPT = 2 << 12; // octal!2
+enumMSG_COPY = 4 << 12; // octal!4
+
+struct msginfo
+{
+int msgpool;
+int msgmap;
+int msgmax;
+int msgmnb;
+int msgmni;
+int msgssz;
+int msgtql;
+ushort msgseg;
+}
+
+version (Alpha) version = GENERICMSQ;
+version (ARM_Any) version = GENERICMSQ;
+version (IA64) version = GENERICMSQ;
+version (IBMZ_Any) version = GENERICMSQ;
+version (RISCV_Any) version = GENERICMSQ;
+
+version (GENERICMSQ)
+{
+// https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/bits/msq-pad.h
+private enum MSQ_PAD_AFTER_TIME = (__WORDSIZE == 32);
+private enum MSQ_PAD_BEFORE_TIME = false;
+}
+else version (HPPA_Any)
+{
+// https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/hppa/bits/msq-pad.h
+private enum MSQ_PAD_AFTER_TIME = false;
+private enum MSQ_PAD_BEFORE_TIME = (__WORDSIZE == 32);
+}
+else version (MIPS_Any)
+{
+// https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/mips/bits/msq-pad.h
+version (LittleEndian)
+{
+private enum MSQ_PAD_AFTER_TIME = (__WORDSIZE == 32);
+private enum MSQ_PAD_BEFORE_TIME = false;
+}
+else
+{
+private enum MSQ_PAD_AFTER_TIME = false;
+private enum MSQ_PAD_BEFORE_TIME = (__WORDSIZE == 32);
+}
+}
+else version (PPC_Any)
+{
+// https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/powerpc/bits/msq-pad.h
+private enum MSQ_PAD_AFTER_TIME = false;
+private enum MSQ_PAD_BEFORE_TIME = (__WORDSIZE == 32);
+}
+else version (SPARC_Any)
+{
+// https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/sparc/bits/msq-pad.h
+private enum MSQ_PAD_AFTER_TIME = false;
+private enum MSQ_PAD_BEFORE_TIME = (__WORDSIZE == 32);
+}
+else version (X86)
+{
+// https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/x86/bits/msq-pad.h
+private enum MSQ_PAD_AFTER_TIME = true;
+private enum MSQ_PAD_BEFORE_TIME = false;
+}
+else version (X86_64)
+{
+// https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/x86/bits/msq-pad.h
+private enum MSQ_PAD_AFTER_TIME = false;
+private enum MSQ_PAD_BEFORE_TIME = false;
+}
+else
+static assert(0, "unimplemented");
+
+// https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/bits/msq.h
+alias msgqnum_t = ulong_t;
+alias msglen_t = ulong_t;
+
+static if (MSQ_PAD_BEFORE_TIME)
+{
+struct msqid_ds
+{
+ipc_permmsg_perm;
+c_ulong __glibc_reserved1;
+time_t msg_stime;
+c_ulong __glibc_reserved2;
+time_t msg_rtime;
+c_ulong __glibc_reserved3;
+time_t msg_ctime;
+ulong_t __msg_cbytes;
+msgqnum_t msg_qnum;
+msglen_tmsg_qbytes;
+pid_t msg_lspid;
+pid_t
Re: [PATCH v4][C][ADA] use function descriptors instead of trampolines in C
Am Sonntag, den 16.12.2018, 09:13 -0700 schrieb Jeff Law: > On 12/16/18 6:45 AM, Uecker, Martin wrote: > > Am Freitag, den 14.12.2018, 18:20 -0700 schrieb Martin Sebor: > > > On 12/14/18 4:36 PM, Jeff Law wrote: > > > > On 12/14/18 3:05 AM, Uecker, Martin wrote: > > > > > > > > > > Am Donnerstag, den 13.12.2018, 16:35 -0700 schrieb Jeff Law: > > > > > > On 12/12/18 11:12 AM, Uecker, Martin wrote: > > > > > > > > > > ... > > > > > > > > > diff --git a/gcc/c/c-objc-common.h b/gcc/c/c-objc-common.h > > > > > > > > > index 78e768c2366..ef039560eb9 100644 > > > > > > > > > --- a/gcc/c/c-objc-common.h > > > > > > > > > +++ b/gcc/c/c-objc-common.h > > > > > > > > > @@ -110,4 +110,7 @@ along with GCC; see the file COPYING3. If > > > > > > > > > not see > > > > > > > > > > > > > > > > > > #undef LANG_HOOKS_TREE_INLINING_VAR_MOD_TYPE_P > > > > > > > > > #define LANG_HOOKS_TREE_INLINING_VAR_MOD_TYPE_P > > > > > > > > > c_vla_unspec_p > > > > > > > > > + > > > > > > > > > +#undef LANG_HOOKS_CUSTOM_FUNCTION_DESCRIPTORS > > > > > > > > > +#define LANG_HOOKS_CUSTOM_FUNCTION_DESCRIPTORS true > > > > > > > > > #endif /* GCC_C_OBJC_COMMON */ > > > > > > > > > > > > > > > > I wonder if we even need the lang hook anymore. ISTM that a > > > > > > > > front-end > > > > > > > > that wants to use the function descriptors can just set > > > > > > > > FUNC_ADDR_BY_DESCRIPTOR and we'd use the function descriptor, > > > > > > > > else we'll > > > > > > > > use the trampoline. Thoughts? > > > > > > > > > > > > > > The lang hook also affects the minimum alignment for function > > > > > > > pointers via the FUNCTION_ALIGNMENT macro (gcc/default.h). This > > > > > > > does > > > > > > > not appear to change the default alignment on any architecture, > > > > > > > but > > > > > > > it causes a failure in i386/gcc.target/i386/attr-aligned.c when > > > > > > > requesting a smaller alignment which is then silently ignored. > > > > > > > > > > > > Ugh. I didn't see that. > > > > > > > > > > The test is new (2019-11-29 Martin Sebor), but one could > > > > > argue that we could simply remove this specific test as 'aligned' > > > > > is only required to increase alignment. Martin? > > > > > > > > The test is meant to test that we do the right thing consistently. If > > > > we're failing with your patch, then that needs to be addressed. > > > > > > I haven't been paying attention here and so I don't know how the test > > > fails after the change. It's meant to verify that attribute aligned > > > successfully reduces the alignment of functions that have not been > > > previously declared with one all the way down to the supported minimum > > > (which is 1 on i386). I agree with Jeff that removing the test would > > > not be right unless the failure is due to some bad assumptions on my > > > part. If it's the built-in that fails that could be due to a bug in > > > it (it's very new). > > > > There is a choice to be made: > > > > Either we activate the lang hook for C, then the minimum alignment for > > functions on is not 1 anymore, because we need one bit to identify the > > descriptors. From a correcntess point of view, this is OK as 'alignas' > > is only required to increase alignment. It is also not really regression > > in my opinion, as it is nowhere documented that one can reduce alignment > > to '1'. The test also has just been added a couple of days ago (if I am > > not mistaken). For these reasons, I think it would be OK to remove the test. > > > > The other option is to decide that having an alignment of only '1' > > is a valuable feature and should be preserved on those platforms which > > support it. I have no idea what this could be good for, but maybe > > there are use cases. In this case, it makes of course sense to keep > > the test. We should then remove the lang hook (as it could never be > > activated for most languages) and instead live with the fact that > > '-fno-trampoline' and using alignof(1) on functions are simply > > incompatible. A warning could be added if the compiler sees > > alignof(1) when -fno-trampoline is active. > > There's certainly targets where 1 byte function alignment is important > from a code space perspective -- it's likely that function descriptors > will never be supported on those targets. But most architectures require a higher alignment anyway. Here is a list of all targets where function alignment is 1 byte: gcc/config/avr/avr.h:#define FUNCTION_BOUNDARY 8 gcc/config/i386/i386.h:#define FUNCTION_BOUNDARY 8 gcc/config/m32c/m32c.h:#define FUNCTION_BOUNDARY 8 gcc/config/mn10300/mn10300.h:#define FUNCTION_BOUNDARY 8 gcc/config/pa/pa.h:#define FUNCTION_BOUNDARY BITS_PER_WORD gcc/config/rl78/rl78.h:#define FUNCTION_BOUNDARY 8 gcc/config/rx/rx.h:#define FUNCTION_BOUNDARY ((rx_cpu_type == RX100 || rx_cpu_type == RX200) ? 4 : 8) From this list only i386 currently adds the target hook and would be affected by this patch. But arm is also
[patch, fortran] Fix PR 85544
Hello world,
the PR pointed out an old regression because the front-end optimization
pass was substituting 2**n with ishift(1,n), where n was an array.
Simply removing the optimization for that case would have been easy,
but also introduced a performance regression.
So, for this, I moved the optimization to trans-*, where it makes more
sense.
Regression-tested. This turned up that one of our tests, mvbits_1.f90,
depends on the behavior that 2**32 is zero. This is certainly not
guaranteed by the standard, but I chose to keep the behavior as not
to introduce any changes in behavior.
This fixes a regression, so I would like to backport to all active
branches if this if possible.
Oh yes, if anybody feels strongly that we should also optimize 32**n
and powers of other powers to two, now is the time to speak up :-)
OK for affected branches?
Regards
Thomas
2018-12-16 Thomas Koenig
PR fortran/85544
* frontend-passes.c (optimize_power): Remove.
(optimize_op): Remove call to optimize_power.
* trans-expr.c (gfc_conv_power_op): Handle cases of 1**integer,
(2|4|8|16) ** integer and (-1) ** integer.
2018-12-16 Thomas Koenig
PR fortran/85544
* gfortran.dg/power_7.f90: New test.
Index: frontend-passes.c
===
--- frontend-passes.c (Revision 267172)
+++ frontend-passes.c (Arbeitskopie)
@@ -1863,84 +1863,6 @@ combine_array_constructor (gfc_expr *e)
return true;
}
-/* Change (-1)**k into 1-ishift(iand(k,1),1) and
- 2**k into ishift(1,k) */
-
-static bool
-optimize_power (gfc_expr *e)
-{
- gfc_expr *op1, *op2;
- gfc_expr *iand, *ishft;
-
- if (e->ts.type != BT_INTEGER)
-return false;
-
- op1 = e->value.op.op1;
-
- if (op1 == NULL || op1->expr_type != EXPR_CONSTANT)
-return false;
-
- if (mpz_cmp_si (op1->value.integer, -1L) == 0)
-{
- gfc_free_expr (op1);
-
- op2 = e->value.op.op2;
-
- if (op2 == NULL)
- return false;
-
- iand = gfc_build_intrinsic_call (current_ns, GFC_ISYM_IAND,
- "_internal_iand", e->where, 2, op2,
- gfc_get_int_expr (e->ts.kind,
- >where, 1));
-
- ishft = gfc_build_intrinsic_call (current_ns, GFC_ISYM_ISHFT,
- "_internal_ishft", e->where, 2, iand,
- gfc_get_int_expr (e->ts.kind,
- >where, 1));
-
- e->value.op.op = INTRINSIC_MINUS;
- e->value.op.op1 = gfc_get_int_expr (e->ts.kind, >where, 1);
- e->value.op.op2 = ishft;
- return true;
-}
- else if (mpz_cmp_si (op1->value.integer, 2L) == 0)
-{
- gfc_free_expr (op1);
-
- op2 = e->value.op.op2;
- if (op2 == NULL)
- return false;
-
- ishft = gfc_build_intrinsic_call (current_ns, GFC_ISYM_ISHFT,
- "_internal_ishft", e->where, 2,
- gfc_get_int_expr (e->ts.kind,
- >where, 1),
- op2);
- *e = *ishft;
- return true;
-}
-
- else if (mpz_cmp_si (op1->value.integer, 1L) == 0)
-{
- op2 = e->value.op.op2;
- if (op2 == NULL)
- return false;
-
- gfc_free_expr (op1);
- gfc_free_expr (op2);
-
- e->expr_type = EXPR_CONSTANT;
- e->value.op.op1 = NULL;
- e->value.op.op2 = NULL;
- mpz_init_set_si (e->value.integer, 1);
- /* Typespec and location are still OK. */
- return true;
-}
-
- return false;
-}
-
/* Recursive optimization of operators. */
static bool
@@ -2001,9 +1923,6 @@ optimize_op (gfc_expr *e)
case INTRINSIC_DIVIDE:
return combine_array_constructor (e) || changed;
-case INTRINSIC_POWER:
- return optimize_power (e);
-
default:
break;
}
Index: trans-expr.c
===
--- trans-expr.c (Revision 267187)
+++ trans-expr.c (Arbeitskopie)
@@ -3056,6 +3056,83 @@ gfc_conv_power_op (gfc_se * se, gfc_expr * expr)
if (gfc_conv_cst_int_power (se, lse.expr, rse.expr))
return;
+ if (INTEGER_CST_P (lse.expr)
+ && TREE_CODE (TREE_TYPE (rse.expr)) == INTEGER_TYPE)
+{
+ wi::tree_to_wide_ref wlhs = wi::to_wide (lse.expr);
+ HOST_WIDE_INT v;
+ v = wlhs.to_shwi ();
+ if (v == 1)
+ {
+ /* 1**something is always 1. */
+ se->expr = build_int_cst (TREE_TYPE (lse.expr), 1);
+ return;
+ }
+ else if (v == 2 || v == 4 || v == 8 || v == 16)
+ {
+ /* 2**n = 1expr = tmp;
+ return;
+ }
+}
+
[PATCH] DWARF: Don't expand hash table when no insert is needed
find_slot_with_hash has
if (insert == INSERT && m_size * 3 <= m_n_elements * 4)
expand ();
which may expand hash table even if no insert is neeed and change hash
table traverse order. When output_macinfo_op is called, all index
strings have been added to hash table by save_macinfo_strings, we
shouldn't expand index string hash table. Otherwise find_slot_with_hash
will expand hash table when hash table has the right size.
Tested on i686 and x86-64. OK for trunk?
Thanks.
H.J.
---
PR debug/79342
* dwarf2out.c (find_AT_string_in_table): Add insert argument
and replace INSERT.
(find_AT_string): Add insert argument defaulting to INSERT
and replace INSERT.
(output_macinfo_op): Pass NO_INSERT to find_AT_string.
---
gcc/dwarf2out.c | 16 +++-
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index b2381056991..2fc88c0c12d 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -4618,12 +4618,13 @@ indirect_string_hasher::equal (indirect_string_node
*x1, const char *x2)
static struct indirect_string_node *
find_AT_string_in_table (const char *str,
-hash_table *table)
+hash_table *table,
+enum insert_option insert = INSERT)
{
struct indirect_string_node *node;
indirect_string_node **slot
-= table->find_slot_with_hash (str, htab_hash_string (str), INSERT);
+= table->find_slot_with_hash (str, htab_hash_string (str), insert);
if (*slot == NULL)
{
node = ggc_cleared_alloc ();
@@ -4640,12 +4641,12 @@ find_AT_string_in_table (const char *str,
/* Add STR to the indirect string hash table. */
static struct indirect_string_node *
-find_AT_string (const char *str)
+find_AT_string (const char *str, enum insert_option insert = INSERT)
{
if (! debug_str_hash)
debug_str_hash = hash_table::create_ggc (10);
- return find_AT_string_in_table (str, debug_str_hash);
+ return find_AT_string_in_table (str, debug_str_hash, insert);
}
/* Add a string attribute value to a DIE. */
@@ -28095,7 +28096,12 @@ output_macinfo_op (macinfo_entry *ref)
break;
case DW_MACRO_define_strp:
case DW_MACRO_undef_strp:
- node = find_AT_string (ref->info);
+ /* NB: output_macinfo_op is called after save_macinfo_strings.
+All index strings have been added to hash table at this point.
+We can't pass INSERT to find_slot_with_hash which may expand
+hash table even if no insert is neeed and change hash table
+traverse order. */
+ node = find_AT_string (ref->info, NO_INSERT);
gcc_assert (node
&& (node->form == DW_FORM_strp
|| node->form == dwarf_FORM (DW_FORM_strx)));
--
2.19.2
[PATCH] PR fortran/85798 -- Check for allocatable components in data statement
The attached patch has been tested on i586-*-freebsd and x86_64-*-freebsd.
If a data-statement-object is a component of a derived type, it checks if
that component is allocatable.
2018-12-16 Steven G . Kargl
PR fortran/85798
* decl.c (gfc_match_data): If a component of a derived type entity
appears in data statement, check that does not have the allocatable
attribute.
2018-12-16 Steven G . Kargl
PR fortran/85798
* gfortran.dg/pr85798.f90: New test.
--
Steve
Index: gcc/fortran/decl.c
===
--- gcc/fortran/decl.c (revision 267189)
+++ gcc/fortran/decl.c (working copy)
@@ -596,6 +596,7 @@ match
gfc_match_data (void)
{
gfc_data *new_data;
+ gfc_expr *e;
match m;
/* Before parsing the rest of a DATA statement, check F2008:c1206. */
@@ -630,6 +631,30 @@ gfc_match_data (void)
gfc_error ("Invalid substring in data-implied-do at %L in DATA "
"statement", _data->var->list->expr->where);
goto cleanup;
+ }
+
+ /* Check for an entity with an allocatable component, which is not
+ allowed. */
+ e = new_data->var->expr;
+ if (e)
+ {
+ bool invalid;
+
+ invalid = false;
+ for (gfc_ref *ref = e->ref; ref; ref = ref->next)
+ if ((ref->type == REF_COMPONENT
+ && ref->u.c.component->attr.allocatable)
+ || (ref->type == REF_ARRAY
+ && e->symtree->n.sym->attr.pointer != 1
+ && ref->u.ar.as && ref->u.ar.as->type == AS_DEFERRED))
+ invalid = true;
+
+ if (invalid)
+ {
+ gfc_error ("Allocatable component or deferred-shaped array "
+ "near %C in DATA statement");
+ goto cleanup;
+ }
}
m = top_val_list (new_data);
Index: gcc/testsuite/gfortran.dg/pr85798.f90
===
--- gcc/testsuite/gfortran.dg/pr85798.f90 (nonexistent)
+++ gcc/testsuite/gfortran.dg/pr85798.f90 (working copy)
@@ -0,0 +1,14 @@
+! { dg-do compile }
+program p
+ type t
+ integer, allocatable :: a(:)
+ end type
+ type u
+ real x
+ type(t) y
+ end type
+ type(t) :: z
+ type(u) :: q
+ data z%a(1) / 789 / ! { dg-error "Allocatable component" }
+ data q%y%a(1) / 789 / ! { dg-error "Allocatable component" }
+end
Re: [patch] Fix bootstrap powerpc*-*-freebsd* targets
On 12.12.18 21:32, Andreas Tobler wrote: this patch fixes bootstrap for my powerpc*-unknown-freebsd* targets. The definition of GNU_USER_DYNAMIC_LINKER was recently moved to linux.h. But the GNU_USER_DYNAMIC_LINKER is still used in rs6000/sysv4.h. So I add an empty definition with guard to cure the bootstrap issue. Ok for trunk? This is another attemtp to fix bootstrap on powerpc*-unknown-freebsd* only. This one is only for FreeBSD and so I can approve and apply myself. Objections? TIA, Andreas 2018-12-16 Andreas Tobler * config/rs6000/freebsd.h: Add an empty definition for GNU_USER_DYNAMIC_LINKER. * config/rs6000/freebsd64.h: Likewise. Index: freebsd.h === --- freebsd.h (revision 267190) +++ freebsd.h (working copy) @@ -72,4 +72,8 @@ /* Use standard DWARF numbering for DWARF debugging information. */ #define RS6000_USE_DWARF_NUMBERING +#ifndef GNU_USER_DYNAMIC_LINKER +#define GNU_USER_DYNAMIC_LINKER "" +#endif + #define POWERPC_FREEBSD Index: freebsd64.h === --- freebsd64.h (revision 267190) +++ freebsd64.h (working copy) @@ -421,4 +421,8 @@ #define USE_LD_AS_NEEDED 1 #endif +#ifndef GNU_USER_DYNAMIC_LINKER +#define GNU_USER_DYNAMIC_LINKER "" +#endif + #define POWERPC_FREEBSD
Re: Fix move_if_noexcept usages in _Hashtable
I reviewed this patch following result of making std::pair piecewise
constructor noexcept:
https://gcc.gnu.org/ml/libstdc++/2018-12/msg00052.html
I restore check of noexcept qualification on move constructor rather
than alloc::construct cause when dealing with std::pairValue> I want to check for noexcept qualification of Value type move
constructor and it doesn't really make sens to check for
alloc::construct when we eventually call allocValue>::construct.
I'll submit this officially once back in stage 1.
François
On 12/4/18 10:43 PM, François Dumont wrote:
On 12/4/18 3:38 PM, Jonathan Wakely wrote:
On 04/12/18 07:45 +0100, François Dumont wrote:
Hi
This patch fix a minor problem with usage of
std::move_if_noexcept. We use it to move node content if move
construtor is noexcept but we eventually use the
allocator_type::construct method which might be slightly different.
I think it is better to check for this method noexcept qualification.
This is likely to pessimize some code, since most allocators do not
have an exception-specification on their construct members.
Perhaps but the Standard mandates to call allocator construct so we
don't have choice. It is surprising to consider value_type move
constructor when we don't know what allocator construct does.
Most users do not change from std::allocator or, even if they do, do
not implement construct so impact should be limited.
Moreover I have added a special overload for nodes containing a
std::pair. It is supposed to allow move semantic in associative
containers where Key is stored as const deleting std::pair move
constructor. In this case we should still move the Value part.
It doesn't work for the moment because the std::pair piecewise
constructor has no noexcept qualification. Is there any plan to add
it ? I think adding it will force including in stl_pair.h,
is it fine ?
No feedback on this point ? Is using std::pair piecewise constructor ok ?
If this evolution is accepted I'll adapt it for _Rb_tree that has
the same problem.
Working on this I also notice that content of initialization_list
is not moved. Is there a plan to make initialization_list iterator
type like move_iterator ? Should containers use
__make_move_iterator_if_noexcept ?
No.
Whether to allow moving from std::initializer_list is an active topic,
see
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p1249r0.html
Ok, nice, allowing emplace build of values would be even better, I'll
have a closer look.
Tested under Linux x86_64 normal mode.
Ok to commit this first step ?
No, this is not suitable for stage 3. It seems too risky.
We can reconsider it during stage 1, but I'd like to see (at least) a
new test showing a bug with the current code. For example, a type with
a move constructor that is noexcept, but when used with a
scoped_allocator_adaptor (which calls something other than the move
constructor) we incorrectly move elements, and lose data when an
exception happens.
Ok, I'll try.
diff --git a/libstdc++-v3/include/bits/hashtable.h b/libstdc++-v3/include/bits/hashtable.h
index 9a12ad399dc..dfb756cec07 100644
--- a/libstdc++-v3/include/bits/hashtable.h
+++ b/libstdc++-v3/include/bits/hashtable.h
@@ -266,8 +266,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_Equal, _H1, _H2, _Hash,
_RehashPolicy, _Traits>;
- using __reuse_or_alloc_node_type =
- __detail::_ReuseOrAllocNode<__node_alloc_type>;
+ template
+ using __alloc_node_gen_t =
+ __detail::_AllocNode<__node_alloc_type, _ConstructF>;
+
+ template
+ using __reuse_or_alloc_node_gen_t =
+ __detail::_ReuseOrAllocNode<__node_alloc_type, _ConstructF>;
// Metaprogramming for picking apart hash caching.
template
@@ -400,9 +405,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
// Assign *this using another _Hashtable instance. Either elements
// are copy or move depends on the _NodeGenerator.
- template
+ template
void
- _M_assign_elements(_Ht&&, const _NodeGenerator&);
+ _M_assign_elements(_Ht&&);
template
void
@@ -500,7 +505,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_Hashtable&
operator=(initializer_list __l)
{
- __reuse_or_alloc_node_type __roan(_M_begin(), *this);
+ __reuse_or_alloc_node_gen_t<_Construct_object_a>
+ __roan(*this, _M_begin());
_M_before_begin._M_nxt = nullptr;
clear();
this->_M_insert_range(__l.begin(), __l.end(), __roan, __unique_keys());
@@ -1047,9 +1053,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_M_rehash_policy = __ht._M_rehash_policy;
__try
{
- _M_assign(__ht,
- [this](const __node_type* __n)
- { return this->_M_allocate_node(__n->_M_v()); });
+ __alloc_node_gen_t<_Construct_object_a> __ang(*this);
+ _M_assign(__ht, __ang);
}
__catch(...)
{
@@ -1064,9 +1069,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
}
// Reuse allocated buckets and nodes.
- _M_assign_elements(__ht,
- [](const
[PATCH] PR fortran/87992 -- trivially stupid patch, but ...
The following patch removes the ICE reported in PR fortran/87992,
and restores the behavior observed with gfortran 7 and 8 (ie,
code compiles).
The PR marks the code with ice-on-invalid-code. I don't use
CLASS in any of code and have never read the standard nor a
Fortran book about CLASS. If the code is invalid, is gfortran
required by a constraint to reject the code. If yes, someone
with CLASS will need to address this PR; otherwise, I will
commit the patch and close it as FIXED.
PS: the patch simply checks for a non-NULL pointer.
Index: gcc/fortran/resolve.c
===
--- gcc/fortran/resolve.c (revision 267190)
+++ gcc/fortran/resolve.c (working copy)
@@ -12313,7 +12313,11 @@ resolve_fl_variable (gfc_symbol *sym, int mp_flag)
{
/* Make sure that character string variables with assumed length are
dummy arguments. */
- e = sym->ts.u.cl->length;
+ if (sym->ts.u.cl)
+ e = sym->ts.u.cl->length;
+ else
+ return false;
+
if (e == NULL && !sym->attr.dummy && !sym->attr.result
&& !sym->ts.deferred && !sym->attr.select_type_temporary
&& !sym->attr.omp_udr_artificial_var)
Index: gcc/testsuite/gfortran.dg/pr87992.f90
===
--- gcc/testsuite/gfortran.dg/pr87992.f90 (nonexistent)
+++ gcc/testsuite/gfortran.dg/pr87992.f90 (working copy)
@@ -0,0 +1,5 @@
+! { dg-do compile }
+subroutine s(x)
+ class(*), allocatable :: x
+ x = ''
+end
--
Steve
Implementation of F2018:18.4 C descriptors and ISO_Fortran_binding.h
The attached is an implementation of ISO_Fortran_binding. Please note
that the ChangeLogs contain no mention of the changes that appear in
the configure files on building in maintainer mode.
The patch only couples to assumed rank and assumed shape formal
arguments of bind_c procedures, via
trans-expr.c(gfc_conv_procedure_call) calls to
gfc_conv_gfc_desc_to_cfi_desc. Such calls in the past would have
generated code that would have been 'invalid' unless typedefs for the
gfortran descriptor were used. For this reason, I believe that this is
safe to commit even at this stage in the release cycle.
It would be nice if ISO_Fortran_binding.h were one of the standard C
includes but I haven't tried to do this yet.
The testing and, indeed, the provision of testcases is incomplete but
I thought this to be a good time for review.
One question for any reviewer is the need for the error messages in
the CFI API functions? Daniel put in the work but they seem
potentially excessive for a library function. In my mind, it is
probably sufficient to return the error code.
Thomas Koenig helped enormously with some build problems for which
thanks. He experienced ICEs associated with malloc/free problems,
which I did not see but believe to have eliminated with the help of
valgrind. All were associated with the use of flexible arrays
Bootstrapped and regtested on FC20/x86_64 - OK for trunk?
Paul
Index: gcc/configure
===
*** gcc/configure (revision 266426)
--- gcc/configure (working copy)
*** foo: .long 25
*** 24393,24404
;;
or1k*-*-*)
conftest_s='
! .section ".tdata","awT",@progbits
! foo:.long 25
! .text
! l.movhi r3, tpoffha(foo)
! l.add r3, r3, r10
! l.lwz r4, tpofflo(foo)(r3)'
tls_first_major=2
tls_first_minor=30
tls_as_opt=--fatal-warnings
--- 24393,24404
;;
or1k*-*-*)
conftest_s='
! .section ".tdata","awT",@progbits
! foo: .long 25
! .text
! l.movhi r3, tpoffha(foo)
! l.add r3, r3, r10
! l.lwz r4, tpofflo(foo)(r3)'
tls_first_major=2
tls_first_minor=30
tls_as_opt=--fatal-warnings
Index: gcc/fortran/trans-array.c
===
*** gcc/fortran/trans-array.c (revision 266426)
--- gcc/fortran/trans-array.c (working copy)
*** gfc_conv_descriptor_rank (tree desc)
*** 293,298
--- 293,314
tree
+ gfc_conv_descriptor_attribute (tree desc)
+ {
+ tree tmp;
+ tree dtype;
+
+ dtype = gfc_conv_descriptor_dtype (desc);
+ tmp = gfc_advance_chain (TYPE_FIELDS (TREE_TYPE (dtype)),
+ GFC_DTYPE_ATTRIBUTE);
+ gcc_assert (tmp!= NULL_TREE
+ && TREE_TYPE (tmp) == short_integer_type_node);
+ return fold_build3_loc (input_location, COMPONENT_REF, TREE_TYPE (tmp),
+ dtype, tmp, NULL_TREE);
+ }
+
+
+ tree
gfc_get_descriptor_dimension (tree desc)
{
tree type, field;
*** gfc_trans_dummy_array_bias (gfc_symbol *
*** 6767,6773
/* Calculate the overall offset, including subreferences. */
! static void
gfc_get_dataptr_offset (stmtblock_t *block, tree parm, tree desc, tree offset,
bool subref, gfc_expr *expr)
{
--- 6783,6789
/* Calculate the overall offset, including subreferences. */
! void
gfc_get_dataptr_offset (stmtblock_t *block, tree parm, tree desc, tree offset,
bool subref, gfc_expr *expr)
{
Index: gcc/fortran/trans-array.h
===
*** gcc/fortran/trans-array.h (revision 266426)
--- gcc/fortran/trans-array.h (working copy)
*** void gfc_conv_tmp_array_ref (gfc_se * se
*** 136,141
--- 136,143
/* Translate a reference to an array temporary. */
void gfc_conv_tmp_ref (gfc_se *);
+ /* Calculate the overall offset, including subreferences. */
+ void gfc_get_dataptr_offset (stmtblock_t*, tree, tree, tree, bool, gfc_expr*);
/* Obtain the span of an array. */
tree gfc_get_array_span (tree, gfc_expr *);
/* Evaluate an array expression. */
*** tree gfc_conv_descriptor_offset_get (tre
*** 167,172
--- 169,175
tree gfc_conv_descriptor_span_get (tree);
tree gfc_conv_descriptor_dtype (tree);
tree gfc_conv_descriptor_rank (tree);
+ tree gfc_conv_descriptor_attribute (tree);
tree gfc_get_descriptor_dimension (tree);
tree gfc_conv_descriptor_stride_get (tree, tree);
tree gfc_conv_descriptor_lbound_get (tree, tree);
Index: gcc/fortran/trans-decl.c
===
*** gcc/fortran/trans-decl.c (revision 266426)
--- gcc/fortran/trans-decl.c (working copy)
*** tree gfor_fndecl_fdate;
*** 114,119
--- 114,121
tree gfor_fndecl_ttynam;
tree gfor_fndecl_in_pack;
tree gfor_fndecl_in_unpack;
+ tree gfor_fndecl_cfi_to_gfc;
+ tree gfor_fndecl_gfc_to_cfi;
tree gfor_fndecl_associated;
tree
Re: [PATCH v4][C][ADA] use function descriptors instead of trampolines in C
On 12/16/18 6:45 AM, Uecker, Martin wrote: > Am Freitag, den 14.12.2018, 18:20 -0700 schrieb Martin Sebor: >> On 12/14/18 4:36 PM, Jeff Law wrote: >>> On 12/14/18 3:05 AM, Uecker, Martin wrote: Am Donnerstag, den 13.12.2018, 16:35 -0700 schrieb Jeff Law: > On 12/12/18 11:12 AM, Uecker, Martin wrote: ... diff --git a/gcc/c/c-objc-common.h b/gcc/c/c-objc-common.h index 78e768c2366..ef039560eb9 100644 --- a/gcc/c/c-objc-common.h +++ b/gcc/c/c-objc-common.h @@ -110,4 +110,7 @@ along with GCC; see the file COPYING3. If not see #undef LANG_HOOKS_TREE_INLINING_VAR_MOD_TYPE_P #define LANG_HOOKS_TREE_INLINING_VAR_MOD_TYPE_P c_vla_unspec_p + +#undef LANG_HOOKS_CUSTOM_FUNCTION_DESCRIPTORS +#define LANG_HOOKS_CUSTOM_FUNCTION_DESCRIPTORS true #endif /* GCC_C_OBJC_COMMON */ >>> >>> I wonder if we even need the lang hook anymore. ISTM that a >>> front-end >>> that wants to use the function descriptors can just set >>> FUNC_ADDR_BY_DESCRIPTOR and we'd use the function descriptor, >>> else we'll >>> use the trampoline. Thoughts? >> >> The lang hook also affects the minimum alignment for function >> pointers via the FUNCTION_ALIGNMENT macro (gcc/default.h). This >> does >> not appear to change the default alignment on any architecture, but >> it causes a failure in i386/gcc.target/i386/attr-aligned.c when >> requesting a smaller alignment which is then silently ignored. > > Ugh. I didn't see that. The test is new (2019-11-29 Martin Sebor), but one could argue that we could simply remove this specific test as 'aligned' is only required to increase alignment. Martin? >>> >>> The test is meant to test that we do the right thing consistently. If >>> we're failing with your patch, then that needs to be addressed. >> >> I haven't been paying attention here and so I don't know how the test >> fails after the change. It's meant to verify that attribute aligned >> successfully reduces the alignment of functions that have not been >> previously declared with one all the way down to the supported minimum >> (which is 1 on i386). I agree with Jeff that removing the test would >> not be right unless the failure is due to some bad assumptions on my >> part. If it's the built-in that fails that could be due to a bug in >> it (it's very new). > > There is a choice to be made: > > Either we activate the lang hook for C, then the minimum alignment for > functions on is not 1 anymore, because we need one bit to identify the > descriptors. From a correcntess point of view, this is OK as 'alignas' > is only required to increase alignment. It is also not really regression > in my opinion, as it is nowhere documented that one can reduce alignment > to '1'. The test also has just been added a couple of days ago (if I am > not mistaken). For these reasons, I think it would be OK to remove the test. > > The other option is to decide that having an alignment of only '1' > is a valuable feature and should be preserved on those platforms which > support it. I have no idea what this could be good for, but maybe > there are use cases. In this case, it makes of course sense to keep > the test. We should then remove the lang hook (as it could never be > activated for most languages) and instead live with the fact that > '-fno-trampoline' and using alignof(1) on functions are simply > incompatible. A warning could be added if the compiler sees > alignof(1) when -fno-trampoline is active. There's certainly targets where 1 byte function alignment is important from a code space perspective -- it's likely that function descriptors will never be supported on those targets. It's also important to remember that not every target which uses function descriptors uses the LSB. On some targets the LSB may switch between modes (arm vs thumb for example). So on those targets the use of descriptors may imply an even larger minimum alignment. Ultimately using function descriptors is an ABI breaking choice and we might declare that function descriptors imply higher function alignments. The ABI implications also likely mean that function descriptors aren't likely going to achieve widespread use. Sigh. jeff
Re: [PATCH] [RFC] PR target/52813 and target/11807
On Sun, Dec 16 2018 at 14:36:26 EET Bernd Edlinger wrote:
> Hi,
>
> if I understood that right, then clobbering sp is and has always been
> ignored.
>
> If that is right, then I would much prefer a warning, that says exactly
> that, because that would also help to understand why removing that clobber
> statement is safe even for old gcc versions.
>
> Since your patch did not actually change the handling of the PIC register,
> that one should of course stay an error.
Thank you. Looks like general consensus is to have a warning. See attached
patch that switches the error to a warning.
Regards,
Dimitar
>From d589ebd7824b4505ab75a2404f49a7c200679545 Mon Sep 17 00:00:00 2001
From: Dimitar Dimitrov
Date: Sun, 16 Dec 2018 10:13:18 +0200
Subject: [PATCH] PR target/52813
Turns out there are existing programs that clobber stack pointer.
To avoid disruption, switch the newly introduced error to a warning.
Tested with:
$ make check-gcc-c RUNTESTFLAGS="i386.exp=pr52813.c "
gcc/ChangeLog:
2018-12-16 Dimitar Dimitrov
* cfgexpand.c (asm_clobber_reg_is_valid): Switch error to warning.
Add clarification why there is a warning.
gcc/testsuite/ChangeLog:
2018-12-16 Dimitar Dimitrov
* gcc.target/i386/pr52813.c (test1): Update warning message.
Signed-off-by: Dimitar Dimitrov
---
gcc/cfgexpand.c | 7 +--
gcc/testsuite/gcc.target/i386/pr52813.c | 2 +-
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 0d04bbcafce..1e44c9a7ad0 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -2872,10 +2872,13 @@ asm_clobber_reg_is_valid (int regno, int nregs, const char *regname)
error ("PIC register clobbered by %qs in %", regname);
is_valid = false;
}
- /* Clobbering the STACK POINTER register is an error. */
+ /* Clobbered STACK POINTER register is not saved/restored by GCC,
+ which is often unexpected by users. See PR52813. */
if (overlaps_hard_reg_set_p (regset, Pmode, STACK_POINTER_REGNUM))
{
- error ("Stack Pointer register clobbered by %qs in %", regname);
+ warning (0, "Stack Pointer register clobbered by %qs in %",
+ regname);
+ warning (0, "GCC has always ignored Stack Pointer % clobbers");
is_valid = false;
}
diff --git a/gcc/testsuite/gcc.target/i386/pr52813.c b/gcc/testsuite/gcc.target/i386/pr52813.c
index 154ebbfc423..644fef15fef 100644
--- a/gcc/testsuite/gcc.target/i386/pr52813.c
+++ b/gcc/testsuite/gcc.target/i386/pr52813.c
@@ -5,5 +5,5 @@
void
test1 (void)
{
- asm volatile ("" : : : "%esp"); /* { dg-error "Stack Pointer register clobbered" } */
+ asm volatile ("" : : : "%esp"); /* { dg-warning "Stack Pointer register clobbered.\+GCC has always ignored Stack Pointer 'asm' clobbers" } */
}
--
2.11.0
Re: [PATCH] PR fortran 88116,88467 -- Catch array constructor errors
On Sun, Dec 16, 2018 at 11:45:11AM +0100, Dominique d'Humières wrote: > Hi Steve, > > The patch works as expected. > > Is "Can\’t " instead of "Can’t " really necessary? > I don't remember how dejagnu handles quotes in the regex expression. "Can\'t" seems to work (for me). If you want to change it, go ahead. Personally, I would prefer "Cannot". -- Steve
Re: [PATCH] x86: Add -march=cascadelake
Thanks for the comments!
Fixed as attached.
Ok for trunk?
Jakub Jelinek 于2018年12月14日周五 下午6:47写道：
>
> On Fri, Dec 14, 2018 at 06:33:37PM +0800, Wei Xiao wrote:
> --- a/gcc/config/i386/driver-i386.c
> +++ b/gcc/config/i386/driver-i386.c
> @@ -832,8 +832,16 @@ const char *host_detect_local_cpu (int argc, const char
> **argv)
> cpu = "skylake";
> break;
> case 0x55:
> - /* Skylake with AVX-512. */
> - cpu = "skylake-avx512";
> + if (has_avx512vnni)
> + {
> + /* Cascade Lake. */
> + cpu = "cascadelake";
> + }
> + else
> + {
> + /* Skylake with AVX-512. */
> + cpu = "skylake-avx512";
> + }
> break;
>
> Just a formatting nit here, if {}s are used, they should be indented
> 2 columns to the right from the if or else and the body of {} should
> be indented by two further columns over {.
> But, in this case, there is another rule, that if the body has a single
> statement, then there shouldn't be {}s around it. Thus just:
> if (has_avx512vnni)
> /* Cascade Lake. */
> cpu = "cascadelake";
> else
> /* Skylake with AVX-512. */
> cpu = "skylake-avx512";
>
> Jakub
cascadelake-v6.diff
Description: Binary data
[patch, fortran, committed] Fix PR 88363
Hi,
I just committed the attached patch as obvious and simple. The problem
was that clobbering of intent(out) clobbered the whole variable, instead
of only the element that was actually clobbered.
There is still some optimization work that can be done here, but I will
defer that to a later date - see PR 41453.
Regards
Thomas
2018-12-16 Thomas Koenig
PF fortran/88364
* trans-expr.c (gfc_conv_expr_reference): Do not add clobber if
the expression contains a reference.
2018-12-16 Thomas Koenig
PR fortran/88364
* intent_out_13.f90: New test.
Added:
trunk/gcc/testsuite/gfortran.dg/intent_out_13.f90
Modified:
trunk/gcc/fortran/ChangeLog
trunk/gcc/fortran/trans-expr.c
trunk/gcc/testsuite/ChangeLog
Index: trans-expr.c
===
--- trans-expr.c (Revision 267172)
+++ trans-expr.c (Arbeitskopie)
@@ -8152,7 +8152,7 @@ gfc_conv_expr_reference (gfc_se * se, gfc_expr * e
gfc_add_block_to_block (>pre, >post);
se->expr = var;
}
- else if (add_clobber)
+ else if (add_clobber && expr->ref == NULL)
{
tree clobber;
tree var;
! { dg-do run }
! PR 88364 -- too much was clobbered on call.
module pr88364
implicit none
type t
integer :: b = -1
integer :: c = 2
end type t
contains
subroutine f1 (x)
integer, intent(out) :: x
x = 5
end subroutine f1
subroutine f2 ()
type(t) :: x
call f1 (x%b)
if (x%b .ne. 5 .or. x%c .ne. 2) stop 1
end subroutine f2
end module pr88364
use pr88364
call f2
end
Re: [PATCH] [RFC] PR target/52813 and target/11807
Hi, if I understood that right, then clobbering sp is and has always been ignored. If that is right, then I would much prefer a warning, that says exactly that, because that would also help to understand why removing that clobber statement is safe even for old gcc versions. Since your patch did not actually change the handling of the PIC register, that one should of course stay an error. Thanks Bernd.
Re: [PATCH v4][C][ADA] use function descriptors instead of trampolines in C
Am Freitag, den 14.12.2018, 18:20 -0700 schrieb Martin Sebor: > On 12/14/18 4:36 PM, Jeff Law wrote: > > On 12/14/18 3:05 AM, Uecker, Martin wrote: > > > > > > Am Donnerstag, den 13.12.2018, 16:35 -0700 schrieb Jeff Law: > > > > On 12/12/18 11:12 AM, Uecker, Martin wrote: > > > > > > ... > > > > > > > diff --git a/gcc/c/c-objc-common.h b/gcc/c/c-objc-common.h > > > > > > > index 78e768c2366..ef039560eb9 100644 > > > > > > > --- a/gcc/c/c-objc-common.h > > > > > > > +++ b/gcc/c/c-objc-common.h > > > > > > > @@ -110,4 +110,7 @@ along with GCC; see the file COPYING3. If > > > > > > > not see > > > > > > > > > > > > > > #undef LANG_HOOKS_TREE_INLINING_VAR_MOD_TYPE_P > > > > > > > #define LANG_HOOKS_TREE_INLINING_VAR_MOD_TYPE_P c_vla_unspec_p > > > > > > > + > > > > > > > +#undef LANG_HOOKS_CUSTOM_FUNCTION_DESCRIPTORS > > > > > > > +#define LANG_HOOKS_CUSTOM_FUNCTION_DESCRIPTORS true > > > > > > > #endif /* GCC_C_OBJC_COMMON */ > > > > > > > > > > > > I wonder if we even need the lang hook anymore. ISTM that a > > > > > > front-end > > > > > > that wants to use the function descriptors can just set > > > > > > FUNC_ADDR_BY_DESCRIPTOR and we'd use the function descriptor, > > > > > > else we'll > > > > > > use the trampoline. Thoughts? > > > > > > > > > > The lang hook also affects the minimum alignment for function > > > > > pointers via the FUNCTION_ALIGNMENT macro (gcc/default.h). This > > > > > does > > > > > not appear to change the default alignment on any architecture, but > > > > > it causes a failure in i386/gcc.target/i386/attr-aligned.c when > > > > > requesting a smaller alignment which is then silently ignored. > > > > > > > > Ugh. I didn't see that. > > > > > > The test is new (2019-11-29 Martin Sebor), but one could > > > argue that we could simply remove this specific test as 'aligned' > > > is only required to increase alignment. Martin? > > > > The test is meant to test that we do the right thing consistently. If > > we're failing with your patch, then that needs to be addressed. > > I haven't been paying attention here and so I don't know how the test > fails after the change. It's meant to verify that attribute aligned > successfully reduces the alignment of functions that have not been > previously declared with one all the way down to the supported minimum > (which is 1 on i386). I agree with Jeff that removing the test would > not be right unless the failure is due to some bad assumptions on my > part. If it's the built-in that fails that could be due to a bug in > it (it's very new). There is a choice to be made: Either we activate the lang hook for C, then the minimum alignment for functions on is not 1 anymore, because we need one bit to identify the descriptors. From a correcntess point of view, this is OK as 'alignas' is only required to increase alignment. It is also not really regression in my opinion, as it is nowhere documented that one can reduce alignment to '1'. The test also has just been added a couple of days ago (if I am not mistaken). For these reasons, I think it would be OK to remove the test. The other option is to decide that having an alignment of only '1' is a valuable feature and should be preserved on those platforms which support it. I have no idea what this could be good for, but maybe there are use cases. In this case, it makes of course sense to keep the test. We should then remove the lang hook (as it could never be activated for most languages) and instead live with the fact that '-fno-trampoline' and using alignof(1) on functions are simply incompatible. A warning could be added if the compiler sees alignof(1) when -fno-trampoline is active. I am happy with both choices. Best, Martin
Re: Fix hashtable node deallocation
Gentle reminder, we still have this issue pending.
* include/bits/hashtable_policy.h
(_Hashtable_alloc<>::_M_deallocate_node_ptr(__node_type*)): New.
(_Hashtable_alloc<>::_M_deallocate_node(__node_type*)): Use latter.
(_ReuseOrAllocNode<>::operator<_Arg>()(_Arg&&)): Likewise.
* libstdc++-v3/testsuite/util/testsuite_allocator.h
(CustomPointerAlloc<>::allocate(size_t, pointer)): Replace by...
(CustomPointerAlloc<>::allocate(size_t, const_void_pointer)): ...this.
François
On 11/29/18 7:08 AM, François Dumont wrote:
I am unclear about this patch, is it accepted ?
On 11/19/18 10:19 PM, François Dumont wrote:
On 11/19/18 1:34 PM, Jonathan Wakely wrote:
On 10/11/18 22:40 +0100, François Dumont wrote:
While working on a hashtable enhancement I noticed that we are not
using the correct method to deallocate node if the constructor
throws in _ReuseOrAllocNode operator(). I had to introduce a new
_M_deallocate_node_ptr for that as node value shall not be destroy
again.
I also check other places and noticed that a __node_type destructor
call was missing.
That's intentional. The type has a trivial destructor, so its storage
can just be reused, we don't need to destroy it.
Ok, do you want to also remove the other call to ~__node_type() then ?
Here is the updated patch and the right ChangeLog entry:
* include/bits/hashtable_policy.h
(_Hashtable_alloc<>::_M_deallocate_node_ptr(__node_type*)): New.
(_Hashtable_alloc<>::_M_deallocate_node(__node_type*)): Use latter.
(_ReuseOrAllocNode<>::operator<_Arg>()(_Arg&&)): Likewise.
(_Hashtable_alloc<>::_M_allocate_node): Add ~__node_type call.
* libstdc++-v3/testsuite/util/testsuite_allocator.h
(CustomPointerAlloc<>::allocate(size_t, pointer)): Replace by...
(CustomPointerAlloc<>::allocate(size_t, const_void_pointer)):
...this.
* testsuite/23_containers/unordered_set/allocator/ext_ptr.cc: Add
check.
Ok to commit ?
François
diff --git a/libstdc++-v3/include/bits/hashtable_policy.h b/libstdc++-v3/include/bits/hashtable_policy.h
index b843c955797..aa4808eab31 100644
--- a/libstdc++-v3/include/bits/hashtable_policy.h
+++ b/libstdc++-v3/include/bits/hashtable_policy.h
@@ -148,8 +148,7 @@ namespace __detail
}
__catch(...)
{
- __node->~__node_type();
- __node_alloc_traits::deallocate(__a, __node, 1);
+ _M_h._M_deallocate_node_ptr(__node);
__throw_exception_again;
}
return __node;
@@ -2116,6 +2115,9 @@ namespace __detail
void
_M_deallocate_node(__node_type* __n);
+ void
+ _M_deallocate_node_ptr(__node_type* __n);
+
// Deallocate the linked list of nodes pointed to by __n
void
_M_deallocate_nodes(__node_type* __n);
@@ -2154,10 +2156,17 @@ namespace __detail
template
void
_Hashtable_alloc<_NodeAlloc>::_M_deallocate_node(__node_type* __n)
+{
+ __node_alloc_traits::destroy(_M_node_allocator(), __n->_M_valptr());
+ _M_deallocate_node_ptr(__n);
+}
+
+ template
+void
+_Hashtable_alloc<_NodeAlloc>::_M_deallocate_node_ptr(__node_type* __n)
{
typedef typename __node_alloc_traits::pointer _Ptr;
auto __ptr = std::pointer_traits<_Ptr>::pointer_to(*__n);
- __node_alloc_traits::destroy(_M_node_allocator(), __n->_M_valptr());
__n->~__node_type();
__node_alloc_traits::deallocate(_M_node_allocator(), __ptr, 1);
}
diff --git a/libstdc++-v3/testsuite/util/testsuite_allocator.h b/libstdc++-v3/testsuite/util/testsuite_allocator.h
index b0fecfb59a3..c18223475c9 100644
--- a/libstdc++-v3/testsuite/util/testsuite_allocator.h
+++ b/libstdc++-v3/testsuite/util/testsuite_allocator.h
@@ -582,7 +582,7 @@ namespace __gnu_test
typedef Ptr void_pointer;
typedef Ptr const_void_pointer;
- pointer allocate(std::size_t n, pointer = {})
+ pointer allocate(std::size_t n, const_void_pointer = {})
{ return pointer(std::allocator::allocate(n)); }
void deallocate(pointer p, std::size_t n)
Re: Optimize streaming in of non-prevailing ipa-parm summaries
On December 16, 2018 1:19:13 PM GMT+01:00, Jan Hubicka wrote: >> On December 15, 2018 10:17:43 PM GMT+01:00, Jan Hubicka > wrote: >> >Hi, >> >this patch is motivated by finding that Firefox ipa summaries >stream-in >> >is fully dominated by streaming edge summaries that are not >necessary >> >because they lead to external calls or are from comdat functions >that >> >are unused. >> > >> >Reading them, populating hashes and later removing them at once >> >streaming is finished increases memory use by about 1GB. This patch >> >simply adds logic to notice such unnecesary summaries early and not >> >put them to memory. This includes ADDR_EXPR that seems to be most >> >common >> >constant values and are easy to rebuild. >> >> Don't you want to go the full way decomposing it to sym + offset? > >Hmm, it may make sense - in stats ADDR_EXPR was most common compared to >refs, but we have easy way to get sym+offset via >get_ref_base_and_offset >and rebuild the tree, right? For addresses should be get_addr_base_and_unit_offset and yes, we can rebuild the thing with & MEM [ + offset] > >Honza
Re: Optimize streaming in of non-prevailing ipa-parm summaries
> On December 15, 2018 10:17:43 PM GMT+01:00, Jan Hubicka > wrote: > >Hi, > >this patch is motivated by finding that Firefox ipa summaries stream-in > >is fully dominated by streaming edge summaries that are not necessary > >because they lead to external calls or are from comdat functions that > >are unused. > > > >Reading them, populating hashes and later removing them at once > >streaming is finished increases memory use by about 1GB. This patch > >simply adds logic to notice such unnecesary summaries early and not > >put them to memory. This includes ADDR_EXPR that seems to be most > >common > >constant values and are easy to rebuild. > > Don't you want to go the full way decomposing it to sym + offset? Hmm, it may make sense - in stats ADDR_EXPR was most common compared to refs, but we have easy way to get sym+offset via get_ref_base_and_offset and rebuild the tree, right? Honza
Speed up fnsummary stream in time
Hi,
this patch does same chage to fnsymmary as I did to ipa-prop to avoid
allocating data we know that will be removed during cgraph merging.
Bootstrapped/regtested x86_64-linux, comitted.
Honza
* ipa-fnsummary.c (analyze_function_body): Do not loeak conds and
size_time_table.
(ipa_fn_summary_generate): Add prevails parameter; do not allocate
data when symbol is not prevailing.
(inline_read_section): Likewise.
Index: ipa-fnsummary.c
===
--- ipa-fnsummary.c (revision 267122)
+++ ipa-fnsummary.c (working copy)
@@ -1990,7 +1990,9 @@ analyze_function_body (struct cgraph_nod
gcc_assert (cfun == my_function);
memset(, 0, sizeof(fbi));
+ vec_free (info->conds);
info->conds = NULL;
+ vec_free (info->size_time_table);
info->size_time_table = NULL;
/* When optimizing and analyzing for IPA inliner, initialize loop optimizer
@@ -3193,28 +3195,46 @@ ipa_fn_summary_generate (void)
/* Write inline summary for edge E to OB. */
static void
-read_ipa_call_summary (struct lto_input_block *ib, struct cgraph_edge *e)
+read_ipa_call_summary (struct lto_input_block *ib, struct cgraph_edge *e,
+ bool prevails)
{
- struct ipa_call_summary *es = ipa_call_summaries->get_create (e);
+ struct ipa_call_summary *es = prevails
+ ? ipa_call_summaries->get_create (e) : NULL;
predicate p;
int length, i;
- es->call_stmt_size = streamer_read_uhwi (ib);
- es->call_stmt_time = streamer_read_uhwi (ib);
- es->loop_depth = streamer_read_uhwi (ib);
+ int size = streamer_read_uhwi (ib);
+ int time = streamer_read_uhwi (ib);
+ int depth = streamer_read_uhwi (ib);
+
+ if (es)
+{
+ es->call_stmt_size = size;
+ es->call_stmt_time = time;
+ es->loop_depth = depth;
+}
bitpack_d bp = streamer_read_bitpack (ib);
- es->is_return_callee_uncaptured = bp_unpack_value (, 1);
+ if (es)
+es->is_return_callee_uncaptured = bp_unpack_value (, 1);
+ else
+bp_unpack_value (, 1);
p.stream_in (ib);
- edge_set_predicate (e, );
+ if (es)
+edge_set_predicate (e, );
length = streamer_read_uhwi (ib);
- if (length)
+ if (length && es && e->possibly_call_in_translation_unit_p ())
{
es->param.safe_grow_cleared (length);
for (i = 0; i < length; i++)
es->param[i].change_prob = streamer_read_uhwi (ib);
}
+ else
+{
+ for (i = 0; i < length; i++)
+ streamer_read_uhwi (ib);
+}
}
@@ -3254,19 +3274,34 @@ inline_read_section (struct lto_file_dec
encoder = file_data->symtab_node_encoder;
node = dyn_cast (lto_symtab_encoder_deref (encoder,
index));
- info = ipa_fn_summaries->get_create (node);
+ info = node->prevailing_p () ? ipa_fn_summaries->get_create (node) :
NULL;
- info->estimated_stack_size
- = info->estimated_self_stack_size = streamer_read_uhwi ();
- info->size = info->self_size = streamer_read_uhwi ();
- info->time = sreal::stream_in ();
+ int stack_size = streamer_read_uhwi ();
+ int size = streamer_read_uhwi ();
+ sreal time = sreal::stream_in ();
+
+ if (info)
+ {
+ info->estimated_stack_size
+ = info->estimated_self_stack_size = stack_size;
+ info->size = info->self_size = size;
+ info->time = time;
+ }
bp = streamer_read_bitpack ();
- info->inlinable = bp_unpack_value (, 1);
- info->fp_expressions = bp_unpack_value (, 1);
+ if (info)
+ {
+ info->inlinable = bp_unpack_value (, 1);
+ info->fp_expressions = bp_unpack_value (, 1);
+ }
+ else
+ {
+ bp_unpack_value (, 1);
+ bp_unpack_value (, 1);
+ }
count2 = streamer_read_uhwi ();
- gcc_assert (!info->conds);
+ gcc_assert (!info || !info->conds);
for (j = 0; j < count2; j++)
{
struct condition c;
@@ -3279,10 +3314,11 @@ inline_read_section (struct lto_file_dec
c.by_ref = bp_unpack_value (, 1);
if (c.agg_contents)
c.offset = streamer_read_uhwi ();
- vec_safe_push (info->conds, c);
+ if (info)
+ vec_safe_push (info->conds, c);
}
count2 = streamer_read_uhwi ();
- gcc_assert (!info->size_time_table);
+ gcc_assert (!info || !info->size_time_table);
for (j = 0; j < count2; j++)
{
struct size_time_entry e;
@@ -3292,19 +3328,23 @@ inline_read_section (struct lto_file_dec
e.exec_predicate.stream_in ();
e.nonconst_predicate.stream_in ();
- vec_safe_push (info->size_time_table, e);
+ if (info)
+ vec_safe_push (info->size_time_table, e);
}
p.stream_in ();
- set_hint_predicate (>loop_iterations, p);
+ if (info)
+
Typos in [Committed] PR fortran/88138 -- can't initialize a derived type
Hi Steve, I think that in decl.c ‘derive’ should be ‘derived’ (twice) and there should be no ’s’ after the dg-error in pr88138.f90. Thanks for the fix. Dominique
Re: [PATCH] PR fortran 88116,88467 -- Catch array constructor errors
Hi Steve, The patch works as expected. Is "Can\’t " instead of "Can’t " really necessary? Thanks for the patch, Dominique
Re: [PATCH] x86: Don't use get_frame_size to finalize stack frame
On 12/15/18, H.J. Lu wrote:
> On Fri, Dec 14, 2018 at 04:04:02PM -0800, H.J. Lu wrote:
>> On Fri, Dec 14, 2018 at 3:24 PM Jeff Law wrote:
>> >
>> > On 12/14/18 4:01 PM, H.J. Lu wrote:
>> > > On Thu, Dec 13, 2018 at 11:11 PM Uros Bizjak
>> > > wrote:
>> > >> On Thu, Dec 13, 2018 at 6:36 PM H.J. Lu
>> > >> wrote:
>> > >>> get_frame_size () returns used stack slots during compilation,
>> > >>> which
>> > >>> may be optimized out later. Since
>> > >>> ix86_find_max_used_stack_alignment
>> > >>> is called by ix86_finalize_stack_frame_flags to check if stack
>> > >>> frame
>> > >>> is required, there is no need to call get_frame_size () which may
>> > >>> give
>> > >>> inaccurate final stack frame size.
>> > >>>
>> > >>> Tested on AVX512 machine configured with
>> > >>>
>> > >>> --with-arch=native --with-cpu=native
>> > >>>
>> > >>> OK for trunk?
>> > >>>
>> > >>>
>> > >>> H.J.
>> > >>> ---
>> > >>> gcc/
>> > >>>
>> > >>> PR target/88483
>> > >>> * config/i386/i386.c (ix86_finalize_stack_frame_flags):
>> > >>> Don't
>> > >>> use get_frame_size ().
>> > >>>
>> > >>> gcc/testsuite/
>> > >>>
>> > >>> PR target/88483
>> > >>> * gcc.target/i386/stackalign/pr88483.c: New test.
>> > >> LGTM, but you know this part of the compiler better than I.
>> > >>
>> > >> Thanks,
>> > >> Uros.
>> > >>
>> > >>> ---
>> > >>> gcc/config/i386/i386.c | 1 -
>> > >>> .../gcc.target/i386/stackalign/pr88483.c| 17
>> > >>> +
>> > >>> 2 files changed, 17 insertions(+), 1 deletion(-)
>> > >>> create mode 100644
>> > >>> gcc/testsuite/gcc.target/i386/stackalign/pr88483.c
>> > >>>
>> > >>> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
>> > >>> index caa701fe242..edc8f4f092e 100644
>> > >>> --- a/gcc/config/i386/i386.c
>> > >>> +++ b/gcc/config/i386/i386.c
>> > >>> @@ -12876,7 +12876,6 @@ ix86_finalize_stack_frame_flags (void)
>> > >>>&& flag_exceptions
>> > >>>&& cfun->can_throw_non_call_exceptions)
>> > >>>&& !ix86_frame_pointer_required ()
>> > >>> - && get_frame_size () == 0
>> > >>>&& ix86_nsaved_sseregs () == 0
>> > >>>&& ix86_varargs_gpr_size + ix86_varargs_fpr_size == 0)
>> > >>> {
>> > >>> diff --git a/gcc/testsuite/gcc.target/i386/stackalign/pr88483.c
>> > >>> b/gcc/testsuite/gcc.target/i386/stackalign/pr88483.c
>> > >>> new file mode 100644
>> > >>> index 000..5aec8fd4cf6
>> > >>> --- /dev/null
>> > >>> +++ b/gcc/testsuite/gcc.target/i386/stackalign/pr88483.c
>> > >>> @@ -0,0 +1,17 @@
>> > >>> +/* { dg-do compile { target i?86-*-* x86_64-*-* } } */
>> > >>> +/* { dg-options "-O2 -mavx2" } */
>> > >>> +
>> > >>> +struct B
>> > >>> +{
>> > >>> + char a[12];
>> > >>> + int b;
>> > >>> +};
>> > >>> +
>> > >>> +struct B
>> > >>> +f2 (void)
>> > >>> +{
>> > >>> + struct B x = {};
>> > >>> + return x;
>> > >>> +}
>> > >>> +
>> > >>> +/* { dg-final { scan-assembler-not
>> > >>> "and\[lq\]?\[^\\n\]*-\[0-9\]+,\[^\\n\]*sp" } } */
>> > >>> --
>> > >>> 2.19.2
>> > >>>
>> > > My fix triggered a latent bug in ix86_find_max_used_stack_alignment.
>> > > Here is the fix. OK for trunk?
>> > >
>> > > Thanks.
>> > >
>> > > -- H.J.
>> > >
>> > >
>> > > 0001-x86-Properly-check-stack-reference.patch
>> > >
>> > > From 83f0b37f287ed198a3b50e2be6b0f7f5c154020e Mon Sep 17 00:00:00
>> > > 2001
>> > > From: "H.J. Lu"
>> > > Date: Fri, 14 Dec 2018 12:21:02 -0800
>> > > Subject: [PATCH] x86: Properly check stack reference
>> > >
>> > > A latent bug in ix86_find_max_used_stack_alignment was uncovered by
>> > > the
>> > > fix for PR target/88483, which caused:
>> > >
>> > > FAIL: gcc.target/i386/incoming-8.c scan-assembler andl[\\t
>> > > ]*\\$-16,[\\t ]*%esp
>> > >
>> > > on i386. ix86_find_max_used_stack_alignment failed to notice stack
>> > > reference via non-stack/frame registers and missed stack alignment
>> > > requirement. We should track all registers which may reference stack
>> > > by checking registers set from stack referencing registers.
>> > >
>> > > Tested on i686 and x86-64 with
>> > >
>> > > --with-arch=native --with-cpu=native
>> > >
>> > > on AVX512 machine. Tested on i686 and x86-64 without
>> > >
>> > > --with-arch=native --with-cpu=native
>> > >
>> > > on x86-64 machine.
>> > >
>> > > PR target/88483
>> > > * config/i386/i386.c (ix86_stack_referenced_p): New function.
>> > > (ix86_find_max_used_stack_alignment): Call
>> > > ix86_stack_referenced_p
>> > > to check if stack is referenced.
>> > > ---
>> > > gcc/config/i386/i386.c | 43
>> > > ++
>> > > 1 file changed, 39 insertions(+), 4 deletions(-)
>> > >
>> > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
>> > > index 4599ca2a7d5..bf93ec3722f 100644
>> > > --- a/gcc/config/i386/i386.c
>> > > +++ b/gcc/config/i386/i386.c
>> > > @@ -12777,6 +12777,18 @@ output_probe_stack_range (rtx reg, rtx end)
>> > >return "";
>> > > }
>>
Re: [PATCH] [RFC] PR target/52813 and target/11807
On Fri, Dec 14 2018 2:52:17 EET Segher Boessenkool wrote: > You need a few tweaks to what you committed. Or just one perhaps: if > flag_pic is not set, you should not check PIC_OFFSET_TABLE_REGNUM, it is > meaningless in that case. I'm not sure if you need to check whether the > register is fixed or not. The flag_pic flag is already checked by the PIC_OFFSET_TABLE_REGNUM macro. It will return INVALID_REGNUM if flag_pic is false, so no error will be printed. Note that the PIC_OFFSET_TABLE_REGNUM behaviour is not changed by my patch. I merely moved the existing check into a separate function. > > But there are many more regs than just the PIC reg and the stack pointer > that you would want to similarly warn about, because overwriting those > registers is just as fatal: the frame pointer, the program counter, etc. > _Most_ fixed registers, but not _all_. > > So maybe it should be a target hook? OTOH that is a lot of work for such > a trivial warning, that isn't very useful anyway (a better warning for > any asm is: "Are you sure?" :-) ) I'll think about a more generic solution. But in light of the several filed PRs I think it's worth having a simple check for SP. Regards, Dimitar
