Patch ping
Hi!
I'd like to ping two patches:
PR c++/79572
- ubsan instrumentation of reference binding to NULL if the reference
is folded into INTEGER_CST with REFERENCE_TYPE
http://gcc.gnu.org/ml/gcc-patches/2017-03/msg01255.html
PR debug/79255
- dwarf2out profiledbootstrap ICE while building gnat;
either the posted patch
http://gcc.gnu.org/ml/gcc-patches/2017-03/msg01257.html
or in gen_decl_die:
case FUNCTION_DECL:
+/* decl is NULL only if when processing a function declaration in
+ BLOCK_NONLOCALIZED_VARS. It is a normal declaration, not an
+ abstract copy of something, so make sure we don't handle it
+ like function inlined into something. */
+if (decl == NULL_TREE)
+ {
+ decl = origin;
+ origin = NULL_TREE;
+ }
or something else (another possibility is to replace all decl
uses in case FUNCTION_DECL with decl_or_origin and
- if (!origin)
- origin = decl_class_context (decl);
+ if (!decl || !origin)
+ origin = decl_class_context (decl_or_origin);
)?
Jakub
Re: [PATCH] Fix expansion of initializer extensions (PR c/80163)
Hi!
On Mon, Mar 27, 2017 at 09:51:27AM -0600, Jeff Law wrote:
> > 2017-03-24 Jakub Jelinek
> >
> > PR c/80163
> > * expr.c : For EXPAND_INITIALIZER determine SIGN_EXTEND
> > vs. ZERO_EXTEND based on signedness of treeop0's type rather than
> > signedness of the result type.
> >
> > * gcc.dg/torture/pr80163.c: New test.
> >
> > --- gcc/expr.c.jj 2017-03-07 09:04:04.0 +0100
> > +++ gcc/expr.c 2017-03-24 12:09:57.729854079 +0100
> > @@ -8333,7 +8333,8 @@ expand_expr_real_2 (sepops ops, rtx targ
> > }
> >
> >else if (modifier == EXPAND_INITIALIZER)
> > - op0 = gen_rtx_fmt_e (unsignedp ? ZERO_EXTEND : SIGN_EXTEND, mode, op0);
> > + op0 = gen_rtx_fmt_e (TYPE_UNSIGNED (TREE_TYPE (treeop0))
> > +? ZERO_EXTEND : SIGN_EXTEND, mode, op0);
> ?!?
>
> Shouldn't the zero/sign extension be derived from the target's type not the
> source types?
No, it needs to be derived from the source operand type, that is exactly the
bug here, we were deriving it from target's type.
> treeop0 is the first source operand, isn't it?
Yes.
If you look at the surrounding code, you can see e.g.:
else if (target == 0)
op0 = convert_to_mode (mode, op0,
TYPE_UNSIGNED (TREE_TYPE
(treeop0)));
else
{
convert_move (target, op0,
TYPE_UNSIGNED (TREE_TYPE (treeop0)));
op0 = target;
}
where the conversion is derived from the operand type, or also:
else if (CONSTANT_P (op0))
{
tree inner_type = TREE_TYPE (treeop0);
machine_mode inner_mode = GET_MODE (op0);
if (inner_mode == VOIDmode)
inner_mode = TYPE_MODE (inner_type);
if (modifier == EXPAND_INITIALIZER)
op0 = lowpart_subreg (mode, op0, inner_mode);
else
op0= convert_modes (mode, inner_mode, op0,
TYPE_UNSIGNED (inner_type));
}
(the lowpart_subreg unconditionally looks problematic too, e.g. if
op0 happens to be scalar int and mode is integral; but let's
deal with it incrementally; perhaps we are always folded and never
reach here with CONST_INTs).
Or consider what kind of extension you need if you have conversions
int -> unsigned long long SIGN_EXTEND
unsigned int-> signed long long ZERO_EXTEND
(and just for completeness):
int -> signed long long (obviously SIGN_EXTEND)
unsigned int-> unsigned long long (obviously ZERO_EXTEND)
Jakub
Re: [C++ PATCH] Add __is_aggregate trait builtin (PR libstdc++/80251)
On 03/30/2017 02:34 PM, Jakub Jelinek wrote: Hi! As discussed with Jon, libstdc++ needs a GCC builtin in order to implement this easily. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2017-03-30 Jakub Jelinek PR libstdc++/80251 c-family/ * c-common.h (enum rid): Add RID_IS_AGGREGATE. * c-common.c (c_common_reswords): Add __is_aggregate trait. * cp-tree.h (enum cp_trait_kind): Add CPTK_IS_AGGREGATE. * cxx-pretty-print.c (pp_cxx_trait_expression): Handle CPTK_IS_AGGREGATE. * semantics.c (trait_expr_value): Handle CPTK_IS_AGGREGATE. Remove extraneous parens. (finish_trait_expr): Handle CPTK_IS_AGGREGATE. * parser.c (cp_parser_primary_expression): Handle RID_IS_AGGREGATE. (cp_parser_trait_expr): Likewise. testsuite/ * g++.dg/ext/is_aggregate.C: New test. OK. jeff
Re: [PATCH] Fix expansion ICE on stores into parts of hard registers (PR middle-end/80173)
On 03/30/2017 02:38 PM, Jakub Jelinek wrote: Hi! On some arches like x86_64 we allow some aggregates in named register variables. If those registers are wider than word, store_bit_field_1 was assuming it must be multiple registers and attempted to pick a word sized subregister of it, which is fine for pseudos, but if the destination is a hard register, sometimes such subreg will fail. If it is a hard register and we know that its mode means a single register (otherwise we wouldn't allow creating a named register variable for it), then that subreg is certainly counter-productive, we just want to store into the whole register. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2017-03-30 Jakub Jelinek PR middle-end/80173 * expmed.c (store_bit_field_1): Don't attempt to create a word subreg out of hard registers wider than word if they have HARD_REGNO_NREGS of 1 for their mode. * gcc.target/i386/pr80173.c: New test. I could easily argue that we shouldn't ever create subregs of hardregs. They should either be rejected or simplified into normal reg expressions. There's been many bugs in the past due to these kinds of problems. This patch clearly fits that general guidance. OK for the trunk and any release branches where you want to backport it. jeff
Re: [PATCH] Fix several buffer overruns in gcov
On 03/31/17 01:27, Nathan Sidwell wrote:
> On 03/30/2017 04:11 PM, Bernd Edlinger wrote:
>> Hi,
>>
>> I'd like to fix a few buffer overruns I have found in the gcov tools.
>> First I noticed that the -x output contains most of the time "ff" bytes,
>> and that when different source files exist in different directories,
>> with same base name the MD5 sum always matches, which results in
>> gcov overwriting the previous result file always, except if -l is given,
>> which makes hashing the file names practically useless.
>>
>> And secondly I wanted to fix potential buffer underflow if a file
>> contains lines with begin with NUL ascii characters, and a out of
>> memory due to always doubling the buffer space, even if the line
>> buffer is not yet filled up.
>>
>>
>> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
>> Is it OK for trunk?
>
> ok. Could you put a comment on the buffer reallocation test about NUL
> defense, thanks!
>
Thanks for the quick response!
I added a comment and commited as r246605:
Index: gcc/gcov.c
===
--- gcc/gcov.c (revision 246604)
+++ gcc/gcov.c (revision 246605)
@@ -2167,7 +2167,7 @@
md5sum_to_hex (const char *sum, char *buffer)
{
for (unsigned i = 0; i < 16; i++)
-sprintf (buffer + (2 * i), "%02x", sum[i]);
+sprintf (buffer + (2 * i), "%02x", (unsigned char)sum[i]);
}
/* Generate an output file name. INPUT_NAME is the canonicalized main
@@ -2216,7 +2216,7 @@
char md5sum_hex[33];
md5_init_ctx (&ctx);
- md5_process_bytes (result, strlen (result), &ctx);
+ md5_process_bytes (src_name, strlen (src_name), &ctx);
md5_finish_ctx (&ctx, md5sum);
md5sum_to_hex (md5sum, md5sum_hex);
free (result);
@@ -2512,14 +2512,20 @@
{
size_t len = strlen (string + pos);
- if (string[pos + len - 1] == '\n')
+ if (len && string[pos + len - 1] == '\n')
{
string[pos + len - 1] = 0;
return string;
}
pos += len;
- string = XRESIZEVEC (char, string, string_len * 2);
- string_len *= 2;
+ /* If the file contains NUL characters or an incomplete
+last line, which can happen more than once in one run,
+we have to avoid doubling the STRING_LEN unnecessarily. */
+ if (pos > string_len / 2)
+ {
+ string_len *= 2;
+ string = XRESIZEVEC (char, string, string_len);
+ }
}
return pos ? string : NULL;
Index: gcc/ChangeLog
===
--- gcc/ChangeLog (revision 246604)
+++ gcc/ChangeLog (revision 246605)
@@ -1,3 +1,10 @@
+2017-03-31 Bernd Edlinger
+
+ * gcov.c (md5sum_to_hex): Fix output of MD5 hex bytes.
+ (make_gcov_file_name): Use the canonical path name for generating
+ the MD5 value.
+ (read_line): Fix handling of files with ascii null bytes.
+
2017-03-30 Matthew Fortune
* config/mips/mips.c (mips_expand_vector_init): Create a const_vector
PS: Could you also please have a look at
https://gcc.gnu.org/ml/gcc-patches/2017-03/msg01434.html
Thanks
Bernd.
[patch, fortran] PR38573 Missing markers for translation
Minor patch to had translation marks.
Regression tested. OK for Trunk?
Jerry
2017-03-30 Jerry DeLisle
PR fortran/38573
* intrinsic.c (gfc_check_intrinsic_standard): Adjust diagnostics.
diff --git a/gcc/fortran/intrinsic.c b/gcc/fortran/intrinsic.c
index 2f60fe8..2e3b250 100644
--- a/gcc/fortran/intrinsic.c
+++ b/gcc/fortran/intrinsic.c
@@ -4575,39 +4575,39 @@ gfc_check_intrinsic_standard (const gfc_intrinsic_sym* isym,
switch (isym->standard)
{
case GFC_STD_F77:
- symstd_msg = "available since Fortran 77";
+ symstd_msg = _("available since Fortran 77");
break;
case GFC_STD_F95_OBS:
- symstd_msg = "obsolescent in Fortran 95";
+ symstd_msg = _("obsolescent in Fortran 95");
break;
case GFC_STD_F95_DEL:
- symstd_msg = "deleted in Fortran 95";
+ symstd_msg = _("deleted in Fortran 95");
break;
case GFC_STD_F95:
- symstd_msg = "new in Fortran 95";
+ symstd_msg = _("new in Fortran 95");
break;
case GFC_STD_F2003:
- symstd_msg = "new in Fortran 2003";
+ symstd_msg = _("new in Fortran 2003");
break;
case GFC_STD_F2008:
- symstd_msg = "new in Fortran 2008";
+ symstd_msg = _("new in Fortran 2008");
break;
case GFC_STD_F2008_TS:
- symstd_msg = "new in TS 29113/TS 18508";
+ symstd_msg = _("new in TS 29113/TS 18508");
break;
case GFC_STD_GNU:
- symstd_msg = "a GNU Fortran extension";
+ symstd_msg = _("a GNU Fortran extension");
break;
case GFC_STD_LEGACY:
- symstd_msg = "for backward compatibility";
+ symstd_msg = _("for backward compatibility");
break;
default:
Re: [PATCH] Fix several buffer overruns in gcov
On 03/30/2017 04:11 PM, Bernd Edlinger wrote: Hi, I'd like to fix a few buffer overruns I have found in the gcov tools. First I noticed that the -x output contains most of the time "ff" bytes, and that when different source files exist in different directories, with same base name the MD5 sum always matches, which results in gcov overwriting the previous result file always, except if -l is given, which makes hashing the file names practically useless. And secondly I wanted to fix potential buffer underflow if a file contains lines with begin with NUL ascii characters, and a out of memory due to always doubling the buffer space, even if the line buffer is not yet filled up. Bootstrapped and reg-tested on x86_64-pc-linux-gnu. Is it OK for trunk? ok. Could you put a comment on the buffer reallocation test about NUL defense, thanks! nathan -- Nathan Sidwell
Re: [RFC] [PATCH v3 0/8] [i386] Use out-of-line stubs for ms_abi pro/epilogues
On 03/30/2017 05:50 PM, Daniel Santos wrote: > I have finally completed all tests for Cygwin and MinGW both 32- & > 64-bit with no additional test failures. There are still 567 tests > failing both pre- and post-patch with error "error while loading shared > libraries: cyggfortran-4.dll: cannot open shared object file: No such > file or directory" in all 32-bit tests even after my (fairly crude) > patch to address that problem. So as a separate issue, I don't yet have > a clean patch set to resolve the windows dll search path issue > (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79867). > > I had to change the test program, as I was dependent upon XSI extensions > which aren't available on Cygwin, so I'll need to repost that. Also, I > had to make one small change in the "aligned SSE MOVs" patch, disabling > it on SEH targets since gcc/config/i386/winnt.c does not currently > support the REG_CFA_EXPRESSION note in its unwind emit. This > optimization primarily targets 64-bit Wine anyway, where stack > realignment is now required. > > Daniel > Very nice, thanks for running the tests. signature.asc Description: OpenPGP digital signature
[patch,MIPS,committed] Fix ICE when expanding MSA constant vectors
This is a fix for a compile failure in gcc.c-torture/compile/pr70240.c with options -O1 -mmsa. The expand code for replicated constant vectors with small immediate values was simply wrong and would never work. This code is not used in the simple case of initialising a variable with a constant vector from C code; so focussed test is non-trivial. Since an existing test triggers the issue then we have basic coverage in place. The whole mips_expand_vector_init function needs a review and possibly a rewrite as on reading through the code I see lots of, at least, confusing logic in there which seems unnecessary. gcc/ * config/mips/mips.c (mips_expand_vector_init): Create a const_vector to initialise a vector register instead of using a const_int. Committed. Thanks, Matthew diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c index d1deb52..dadfcc4 100644 --- a/gcc/config/mips/mips.c +++ b/gcc/config/mips/mips.c @@ -21757,11 +21757,12 @@ mips_expand_vector_init (rtx target, rtx vals) case V8HImode: case V4SImode: case V2DImode: - emit_move_insn (target, same); + temp = gen_rtx_CONST_VECTOR (vmode, XVEC (vals, 0)); + emit_move_insn (target, temp); return; default: - break; + gcc_unreachable (); } } temp = gen_reg_rtx (imode);
[PATCH,MIPS,committed] Fix pr52125.c test when built as -mno-abicalls -mabi=64
pr52125.c verifies that orphaned %hi relocs are deleted if they feed
an inline asm statement that never emits the %lo part. Orphaned %hi
relocs are only strictly a problem for o32 but are eliminated for
any ABI as long as 32-bit addressing is in use so force -msym32 as well
as require absolute addressing. This is necessary because -msym32 is
only applied for n64 as part of downgrading -mabicalls to absolute
addressing. I.e. this test was broken for bare metal n64 configs.
gcc/testsuite/
* gcc.target/mips/pr52125.c: Add -msym32.
diff --git a/gcc/testsuite/gcc.target/mips/pr52125.c
b/gcc/testsuite/gcc.target/mips/pr52125.c
index 2ac8067..46df940 100644
--- a/gcc/testsuite/gcc.target/mips/pr52125.c
+++ b/gcc/testsuite/gcc.target/mips/pr52125.c
@@ -1,4 +1,4 @@
-/* { dg-options "-mno-gpopt addressing=absolute" } */
+/* { dg-options "-mno-gpopt -msym32 addressing=absolute" } */
int a, b, c, d;
Re: [C++ PATCH] Disable VLA initialization? (PR sanitizer/79993)
On 03/30/2017 02:24 PM, Jakub Jelinek wrote: On Thu, Mar 30, 2017 at 10:21:09AM -0600, Martin Sebor wrote: I don't think rejecting all VLA initialization just to avoid an Asan ICE with string literals is a good way to solve the problem. For one, it will break working programs that rely on The Asan ICE can be easily worked around, the reason I don't want to do that is that this is the second time this broke something; the FE should not pass a VLA static constant (STRING_CST in this case) to the middle-end, VLA objects can be by definition only automatic variables or something on the heap, never in the data/rodata sections. Plus, while for VLA initialized from arrays etc. we copy the initializer and zero initialize the rest, for VLA initialization by STRING_CST we just copy over the STRING_CST and the rest is uninitialized. I agree that's a bug, one that should be fixed, but ion my view it's not serious enough to justify disabling the feature altogether. If every feature with a bug in it or that happens to trigger an ICE with some combination of command line options had to be disabled there wouldn't be much left. As I said, a patch that fixes this exists. I ran out of time to resubmit it for GCC 7 but I plan to do it for GCC 8. It be quite unfriendly to users to have GCC 7 reject working code that's accepted by GCC 6 only to have GCC 8 accept it again. I know you have concerns about the runtime costs of the patch and I'm willing to listen and work with you to minimize the performance impact. Martin
[PATCH] Fix expansion ICE on stores into parts of hard registers (PR middle-end/80173)
Hi!
On some arches like x86_64 we allow some aggregates in named register
variables. If those registers are wider than word, store_bit_field_1
was assuming it must be multiple registers and attempted to pick
a word sized subregister of it, which is fine for pseudos, but if the
destination is a hard register, sometimes such subreg will fail.
If it is a hard register and we know that its mode means a single
register (otherwise we wouldn't allow creating a named register variable for
it), then that subreg is certainly counter-productive, we just want to store
into the whole register.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2017-03-30 Jakub Jelinek
PR middle-end/80173
* expmed.c (store_bit_field_1): Don't attempt to create
a word subreg out of hard registers wider than word if they
have HARD_REGNO_NREGS of 1 for their mode.
* gcc.target/i386/pr80173.c: New test.
--- gcc/expmed.c.jj 2017-01-01 12:45:39.0 +0100
+++ gcc/expmed.c2017-03-30 14:51:01.814449691 +0200
@@ -965,8 +965,14 @@ store_bit_field_1 (rtx str_rtx, unsigned
}
/* If OP0 is a multi-word register, narrow it to the affected word.
- If the region spans two words, defer to store_split_bit_field. */
- if (!MEM_P (op0) && GET_MODE_SIZE (GET_MODE (op0)) > UNITS_PER_WORD)
+ If the region spans two words, defer to store_split_bit_field.
+ Don't do this if op0 is a single hard register wider than word
+ such as a float or vector register. */
+ if (!MEM_P (op0)
+ && GET_MODE_SIZE (GET_MODE (op0)) > UNITS_PER_WORD
+ && (!REG_P (op0)
+ || !HARD_REGISTER_P (op0)
+ || HARD_REGNO_NREGS (REGNO (op0), GET_MODE (op0)) != 1))
{
if (bitnum % BITS_PER_WORD + bitsize > BITS_PER_WORD)
{
--- gcc/testsuite/gcc.target/i386/pr80173.c.jj 2017-03-30 15:05:49.600889474
+0200
+++ gcc/testsuite/gcc.target/i386/pr80173.c 2017-03-30 15:05:33.0
+0200
@@ -0,0 +1,22 @@
+/* PR middle-end/80173 */
+/* { dg-do compile { target lp64 } } */
+/* { dg-options "-O2 -ffixed-xmm7" } */
+
+typedef int V __attribute__ ((vector_size (2 * sizeof (int;
+
+struct U { V a; V b; };
+
+int
+foo (int i)
+{
+ register struct U u asm ("xmm7") = {{-1, 0}, {-1, 0}};
+ return u.b[i];
+}
+
+int
+bar (int i)
+{
+ register struct U u asm ("xmm7");
+ u = (struct U) {{-1, 0}, {-1, 0}};
+ return u.b[i];
+}
Jakub
[wwwdocs] Adjust to the new location of the C++ ABI
For gcc-4.0/ and faq.html I did adjust the link, for gcc-3.2/ I figured we can as well avoid it. Applied. (Jonathan, I'm going to take care of the libstdc++/doc links as well in case you wonder.) Gerald Index: faq.html === RCS file: /cvs/gcc/wwwdocs/htdocs/faq.html,v retrieving revision 1.224 diff -u -r1.224 faq.html --- faq.html3 Feb 2017 07:43:57 - 1.224 +++ faq.html29 Mar 2017 09:10:23 - @@ -428,7 +428,7 @@ For more details about the way that GCC implements these and other C++ features, please read the http://mentorembedded.github.io/cxx-abi/";>C++ ABI specification. +href="https://itanium-cxx-abi.github.io/cxx-abi/";>C++ ABI specification. Note the std::type_info objects which must be resolved all begin with "_ZTS". Refer to ld's documentation for a description of the "-E" & Index: gcc-3.2/c++-abi.html === RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-3.2/c++-abi.html,v retrieving revision 1.9 retrieving revision 1.11 diff -u -r1.9 -r1.11 --- gcc-3.2/c++-abi.html1 Feb 2017 20:18:54 - 1.9 +++ gcc-3.2/c++-abi.html30 Mar 2017 20:36:53 - 1.11 @@ -9,10 +9,9 @@ The main point of the GCC 3.2 release is to have a relatively stable and common C++ ABI for GNU/Linux and BSD usage, following -the documentation at -http://mentorembedded.github.io/cxx-abi/";>http://mentorembedded.github.io/cxx-abi/. +the Itanium C++ ABI. -Unfortunately this means that GCC 3.2 is incompatible with GCC 3.0 +Unfortunately this means that GCC 3.2 is incompatible with GCC 3.0 and GCC 3.1 releases. Index: gcc-4.0/changes.html === RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.0/changes.html,v retrieving revision 1.69 diff -u -r1.69 changes.html --- gcc-4.0/changes.html19 Mar 2017 19:07:26 - 1.69 +++ gcc-4.0/changes.html29 Mar 2017 09:10:24 - @@ -182,7 +182,7 @@ -fvisibility option. The compiler now uses the library interface specified by the http://mentorembedded.github.io/cxx-abi/";>C++ ABI for +href="https://itanium-cxx-abi.github.io/cxx-abi/";>C++ ABI for thread-safe initialization of function-scope static variables. Most users should leave this alone, but embedded programmers may want to disable this by specifying
[C++ PATCH] Add __is_aggregate trait builtin (PR libstdc++/80251)
Hi!
As discussed with Jon, libstdc++ needs a GCC builtin in order to implement
this easily.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2017-03-30 Jakub Jelinek
PR libstdc++/80251
c-family/
* c-common.h (enum rid): Add RID_IS_AGGREGATE.
* c-common.c (c_common_reswords): Add __is_aggregate trait.
* cp-tree.h (enum cp_trait_kind): Add CPTK_IS_AGGREGATE.
* cxx-pretty-print.c (pp_cxx_trait_expression): Handle
CPTK_IS_AGGREGATE.
* semantics.c (trait_expr_value): Handle CPTK_IS_AGGREGATE.
Remove extraneous parens.
(finish_trait_expr): Handle CPTK_IS_AGGREGATE.
* parser.c (cp_parser_primary_expression): Handle RID_IS_AGGREGATE.
(cp_parser_trait_expr): Likewise.
testsuite/
* g++.dg/ext/is_aggregate.C: New test.
--- gcc/c-family/c-common.h.jj 2017-02-24 21:39:13.0 +0100
+++ gcc/c-family/c-common.h 2017-03-30 11:54:35.289989887 +0200
@@ -162,8 +162,8 @@ enum rid
RID_HAS_TRIVIAL_CONSTRUCTOR, RID_HAS_TRIVIAL_COPY,
RID_HAS_TRIVIAL_DESTRUCTOR, RID_HAS_UNIQUE_OBJ_REPRESENTATIONS,
RID_HAS_VIRTUAL_DESTRUCTOR,
- RID_IS_ABSTRACT, RID_IS_BASE_OF,
- RID_IS_CLASS,
+ RID_IS_ABSTRACT, RID_IS_AGGREGATE,
+ RID_IS_BASE_OF, RID_IS_CLASS,
RID_IS_EMPTY,RID_IS_ENUM,
RID_IS_FINAL,RID_IS_LITERAL_TYPE,
RID_IS_POD, RID_IS_POLYMORPHIC,
--- gcc/c-family/c-common.c.jj 2017-03-29 07:11:13.0 +0200
+++ gcc/c-family/c-common.c 2017-03-30 11:54:06.551365199 +0200
@@ -403,6 +403,7 @@ const struct c_common_resword c_common_r
{ "__inline",RID_INLINE, 0 },
{ "__inline__", RID_INLINE, 0 },
{ "__is_abstract", RID_IS_ABSTRACT, D_CXXONLY },
+ { "__is_aggregate", RID_IS_AGGREGATE, D_CXXONLY },
{ "__is_base_of",RID_IS_BASE_OF, D_CXXONLY },
{ "__is_class", RID_IS_CLASS, D_CXXONLY },
{ "__is_empty", RID_IS_EMPTY, D_CXXONLY },
--- gcc/cp/cp-tree.h.jj 2017-03-29 07:11:18.0 +0200
+++ gcc/cp/cp-tree.h2017-03-30 11:56:39.036370331 +0200
@@ -728,6 +728,7 @@ enum cp_trait_kind
CPTK_HAS_UNIQUE_OBJ_REPRESENTATIONS,
CPTK_HAS_VIRTUAL_DESTRUCTOR,
CPTK_IS_ABSTRACT,
+ CPTK_IS_AGGREGATE,
CPTK_IS_BASE_OF,
CPTK_IS_CLASS,
CPTK_IS_EMPTY,
--- gcc/cp/cxx-pretty-print.c.jj2017-01-01 12:45:44.0 +0100
+++ gcc/cp/cxx-pretty-print.c 2017-03-30 11:57:10.924952663 +0200
@@ -2585,6 +2585,9 @@ pp_cxx_trait_expression (cxx_pretty_prin
case CPTK_IS_ABSTRACT:
pp_cxx_ws_string (pp, "__is_abstract");
break;
+case CPTK_IS_AGGREGATE:
+ pp_cxx_ws_string (pp, "__is_aggregate");
+ break;
case CPTK_IS_BASE_OF:
pp_cxx_ws_string (pp, "__is_base_of");
break;
--- gcc/cp/semantics.c.jj 2017-03-22 19:53:09.0 +0100
+++ gcc/cp/semantics.c 2017-03-30 11:59:24.240206534 +0200
@@ -9144,7 +9144,10 @@ trait_expr_value (cp_trait_kind kind, tr
return type_has_unique_obj_representations (type1);
case CPTK_IS_ABSTRACT:
- return (ABSTRACT_CLASS_TYPE_P (type1));
+ return ABSTRACT_CLASS_TYPE_P (type1);
+
+case CPTK_IS_AGGREGATE:
+ return CP_AGGREGATE_TYPE_P (type1);
case CPTK_IS_BASE_OF:
return (NON_UNION_CLASS_TYPE_P (type1) && NON_UNION_CLASS_TYPE_P (type2)
@@ -9152,34 +9155,34 @@ trait_expr_value (cp_trait_kind kind, tr
|| DERIVED_FROM_P (type1, type2)));
case CPTK_IS_CLASS:
- return (NON_UNION_CLASS_TYPE_P (type1));
+ return NON_UNION_CLASS_TYPE_P (type1);
case CPTK_IS_EMPTY:
- return (NON_UNION_CLASS_TYPE_P (type1) && CLASSTYPE_EMPTY_P (type1));
+ return NON_UNION_CLASS_TYPE_P (type1) && CLASSTYPE_EMPTY_P (type1);
case CPTK_IS_ENUM:
- return (type_code1 == ENUMERAL_TYPE);
+ return type_code1 == ENUMERAL_TYPE;
case CPTK_IS_FINAL:
- return (CLASS_TYPE_P (type1) && CLASSTYPE_FINAL (type1));
+ return CLASS_TYPE_P (type1) && CLASSTYPE_FINAL (type1);
case CPTK_IS_LITERAL_TYPE:
- return (literal_type_p (type1));
+ return literal_type_p (type1);
case CPTK_IS_POD:
- return (pod_type_p (type1));
+ return pod_type_p (type1);
case CPTK_IS_POLYMORPHIC:
- return (CLASS_TYPE_P (type1) && TYPE_POLYMORPHIC_P (type1));
+ return CLASS_TYPE_P (type1) && TYPE_POLYMORPHIC_P (type1);
case CPTK_IS_SAME_AS:
return same_type_p (type1, type2);
case CPTK_IS_STD_LAYOUT:
- return (std_layout_type_p (type1));
+ return std_layout_type_p (type1);
case CPTK_IS_TRIVIAL:
- return (trivial_type_p (type1));
+ return trivial_type_p (type1);
case CPTK_IS_TRIVIALLY_ASSIGNABLE:
return is_trivially_xible (MODIFY_EXPR, type1, type2);
@@ -9188,10 +9191,10 @@ trait_expr_value (cp_trait_kind kind, tr
return is_trivially_xible (INIT_EXPR, type1, type2);
case CPTK_IS_
[committed] Small OpenMP diagnostics improvement (PR translation/80189)
Hi!
This makes it clearer to translators and users that parallel, teams etc.
are keywords.
Bootstrapped/regtested on x86_64-linux and i686-linux, committed to trunk.
2017-03-30 Jakub Jelinek
PR translation/80189
* gimplify.c (omp_default_clause): Use %qs instead of %s in
diagnostic messages.
testsuite/
* g++.dg/gomp/predetermined-1.C: Adjust expected diagnostics.
* g++.dg/gomp/sharing-1.C: Likewise.
* gfortran.dg/gomp/pr44536.f90: Likewise.
* gfortran.dg/gomp/pr44036-1.f90: Likewise.
* gfortran.dg/gomp/sharing-3.f90: Likewise.
* gfortran.dg/gomp/crayptr3.f90: Likewise.
* gfortran.dg/gomp/pr33439.f90: Likewise.
* gfortran.dg/gomp/appendix-a/a.24.1.f90: Likewise.
* gfortran.dg/gomp/sharing-1.f90: Likewise.
* gfortran.dg/gomp/sharing-2.f90: Likewise.
* gcc.dg/gomp/appendix-a/a.24.1.c: Likewise.
* gcc.dg/gomp/sharing-1.c: Likewise.
--- gcc/gimplify.c.jj 2017-03-23 15:49:56.0 +0100
+++ gcc/gimplify.c 2017-03-30 10:51:13.807725987 +0200
@@ -6847,9 +6847,9 @@ omp_default_clause (struct gimplify_omp_
else
gcc_unreachable ();
- error ("%qE not specified in enclosing %s",
+ error ("%qE not specified in enclosing %qs",
DECL_NAME (lang_hooks.decls.omp_report_decl (decl)), rtype);
- error_at (ctx->location, "enclosing %s", rtype);
+ error_at (ctx->location, "enclosing %qs", rtype);
}
/* FALLTHRU */
case OMP_CLAUSE_DEFAULT_SHARED:
--- gcc/testsuite/g++.dg/gomp/predetermined-1.C.jj 2008-09-05
12:55:06.0 +0200
+++ gcc/testsuite/g++.dg/gomp/predetermined-1.C 2017-03-30 12:08:20.688180275
+0200
@@ -19,15 +19,15 @@ const A foo (const A d, const C e)
bar (&a);
#pragma omp parallel default (none)
bar (&b);
- #pragma omp parallel default (none) // { dg-error "enclosing parallel" }
+ #pragma omp parallel default (none) // { dg-error "enclosing 'parallel'" }
bar (&c); // { dg-error "not specified" }
#pragma omp parallel default (none)
bar (&d);
- #pragma omp parallel default (none) // { dg-error "enclosing parallel" }
+ #pragma omp parallel default (none) // { dg-error "enclosing 'parallel'" }
bar (&e); // { dg-error "not specified" }
#pragma omp parallel default (none)
bar (&f);
- #pragma omp parallel default (none) // { dg-error "enclosing parallel" }
+ #pragma omp parallel default (none) // { dg-error "enclosing 'parallel'" }
bar (&g); // { dg-error "not specified" }
return f;
}
--- gcc/testsuite/g++.dg/gomp/sharing-1.C.jj2010-07-28 10:35:53.0
+0200
+++ gcc/testsuite/g++.dg/gomp/sharing-1.C 2017-03-30 12:07:54.785519542
+0200
@@ -52,7 +52,7 @@ main (void)
*p = 7;
s = 6;
l = 0;
-#pragma omp parallel for /* { dg-error "enclosing parallel" } */ \
+#pragma omp parallel for /* { dg-error "enclosing 'parallel'" } */ \
default (none) private (p) shared (s)
for (i = 0; i < 64; i++)
{
--- gcc/testsuite/gfortran.dg/gomp/pr44536.f90.jj 2010-06-16
10:23:58.0 +0200
+++ gcc/testsuite/gfortran.dg/gomp/pr44536.f90 2017-03-30 12:08:56.148715822
+0200
@@ -4,7 +4,7 @@
subroutine foo (a, i, j)
integer, dimension(:) :: a
integer :: i, j
-!$omp parallel default(none) shared(i, j) ! { dg-error "enclosing
parallel" }
+!$omp parallel default(none) shared(i, j) ! { dg-error "enclosing
'parallel'" }
j=a(i) ! { dg-error "not specified in"
}
!$omp end parallel
end subroutine
--- gcc/testsuite/gfortran.dg/gomp/pr44036-1.f90.jj 2010-05-17
07:51:59.0 +0200
+++ gcc/testsuite/gfortran.dg/gomp/pr44036-1.f902017-03-30
12:10:01.746856635 +0200
@@ -11,14 +11,14 @@ subroutine foo(a, b)
!$omp parallel default(none) private (x)
x = a(4)
!$omp end parallel
-!$omp parallel default(none) private (x) ! { dg-error "enclosing
parallel" }
+!$omp parallel default(none) private (x) ! { dg-error "enclosing
'parallel'" }
x = b(5) ! { dg-error "not specified in"
}
!$omp end parallel
!$omp parallel default(none) private (x)
x = c(6)
!$omp end parallel
d => a
-!$omp parallel default(none) private (x) ! { dg-error "enclosing
parallel" }
+!$omp parallel default(none) private (x) ! { dg-error "enclosing
'parallel'" }
x = d(7) ! { dg-error "not specified in"
}
!$omp end parallel
end
--- gcc/testsuite/gfortran.dg/gomp/sharing-3.f90.jj 2010-06-16
10:23:58.0 +0200
+++ gcc/testsuite/gfortran.dg/gomp/sharing-3.f902017-03-30
12:11:30.198698115 +0200
@@ -25,7 +25,7 @@ subroutine foo (vara, varb, varc, vard,
vard(1) = 1
vare(1) = 1
!$omp end parallel
- !$omp parallel default(none) ! { dg-error "enclos
[committed] Fix OMP_DISPLAY_ENV without {,G}OMP_STACKSIZE
Hi!
We print an uninitialized variable if OMP_DISPLAY_ENV=true is
in the environment, but {,G}OMP_STACKSIZE is not.
Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
committed to trunk.
2017-03-30 Jakub Jelinek
* env.c (initialize_env): Initialize stacksize to 0.
--- libgomp/env.c.jj2017-01-01 12:45:52.0 +0100
+++ libgomp/env.c 2017-03-30 16:07:29.800247913 +0200
@@ -1187,7 +1187,7 @@ handle_omp_display_env (unsigned long st
static void __attribute__((constructor))
initialize_env (void)
{
- unsigned long thread_limit_var, stacksize;
+ unsigned long thread_limit_var, stacksize = 0;
int wait_policy;
/* Do a compile time check that mkomp_h.pl did good job. */
Jakub
Re: [C++ PATCH] Disable VLA initialization? (PR sanitizer/79993)
On Thu, Mar 30, 2017 at 10:21:09AM -0600, Martin Sebor wrote: > I don't think rejecting all VLA initialization just to avoid > an Asan ICE with string literals is a good way to solve the > problem. For one, it will break working programs that rely on The Asan ICE can be easily worked around, the reason I don't want to do that is that this is the second time this broke something; the FE should not pass a VLA static constant (STRING_CST in this case) to the middle-end, VLA objects can be by definition only automatic variables or something on the heap, never in the data/rodata sections. Plus, while for VLA initialized from arrays etc. we copy the initializer and zero initialize the rest, for VLA initialization by STRING_CST we just copy over the STRING_CST and the rest is uninitialized. Jakub
Re: [PATCH, rs6000] Fix PR target/80246, DFP builtins using the wrong types
On 3/30/17 12:54 PM, Peter Bergner wrote:
> On 3/30/17 12:15 PM, Segher Boessenkool wrote:
> +/* { dg-final { scan-assembler-not "drintn\\." } } */
> +/* { dg-final { scan-assembler-not "drintnq\\." } } */
> +/* { dg-final { scan-assembler-not "dctfix" } } */
> +/* { dg-final { scan-assembler-not "dctfixq" } } */
If there is no "dctfix" there surely is no "dctfixq" either (i.e., your
regexen aren't very tight).
>>>
>>> Ahh, true. I suppose I could also just look for "drintn" too,
>>> since that would catch both drintn. and drintnq., ok with that
>>> change?
>>
>> Please add a comment what instructions each regex is supposed to match, then?
>> Okay with such a change.
>
> Actually, the following is probably better. I'll go with this unless
> you object.
>
> /* { dg-final { scan-assembler-not "drintn\[q\]\." } } */
> /* { dg-final { scan-assembler-not "dctfix\[q\]" } } */
> /* { dg-final { scan-assembler-not "dcffix\[q\]" } } */
Ok, committed the above change to trunk and GCC 6 and GCC5 release
branches, along with a comment like you wanted. Thanks.
Peter
[PATCH] Fix several buffer overruns in gcov
Hi,
I'd like to fix a few buffer overruns I have found in the gcov tools.
First I noticed that the -x output contains most of the time "ff" bytes,
and that when different source files exist in different directories,
with same base name the MD5 sum always matches, which results in
gcov overwriting the previous result file always, except if -l is given,
which makes hashing the file names practically useless.
And secondly I wanted to fix potential buffer underflow if a file
contains lines with begin with NUL ascii characters, and a out of
memory due to always doubling the buffer space, even if the line
buffer is not yet filled up.
Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?
Thanks
Bernd.
2017-03-30 Bernd Edlinger
* gcov.c (md5sum_to_hex): Fix output of MD5 hex bytes.
(make_gcov_file_name): Use the canonical path name for generating
the MD5 value.
(read_line): Fix handling of files with ascii null bytes.
Index: gcc/gcov.c
===
--- gcc/gcov.c (revision 246571)
+++ gcc/gcov.c (working copy)
@@ -2167,7 +2167,7 @@ static void
md5sum_to_hex (const char *sum, char *buffer)
{
for (unsigned i = 0; i < 16; i++)
-sprintf (buffer + (2 * i), "%02x", sum[i]);
+sprintf (buffer + (2 * i), "%02x", (unsigned char)sum[i]);
}
/* Generate an output file name. INPUT_NAME is the canonicalized main
@@ -2216,7 +2216,7 @@ make_gcov_file_name (const char *input_name, const
char md5sum_hex[33];
md5_init_ctx (&ctx);
- md5_process_bytes (result, strlen (result), &ctx);
+ md5_process_bytes (src_name, strlen (src_name), &ctx);
md5_finish_ctx (&ctx, md5sum);
md5sum_to_hex (md5sum, md5sum_hex);
free (result);
@@ -2512,14 +2512,17 @@ read_line (FILE *file)
{
size_t len = strlen (string + pos);
- if (string[pos + len - 1] == '\n')
+ if (len && string[pos + len - 1] == '\n')
{
string[pos + len - 1] = 0;
return string;
}
pos += len;
- string = XRESIZEVEC (char, string, string_len * 2);
- string_len *= 2;
+ if (pos > string_len / 2)
+ {
+ string_len *= 2;
+ string = XRESIZEVEC (char, string, string_len);
+ }
}
return pos ? string : NULL;
C++ PATCH for c++/80241, ICE with alignas pack expansion
This fix is similar to c++/79653: if something went wrong during
make_pack_expansion, return error_mark instead of building up the attribute
otherwise we end up with "gnu aligned <<>>" and that would mean crashing
later on at a lot of spots.
Also, dump_expr wasn't able to print TREE_LISTs. It's easy to do so using
dump_expr_list, so that we print '__alignof__ (A)' instead of ugly
'#'tree_list' not supported by dump_expr#'.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2017-03-30 Marek Polacek
PR c++/80241 - ICE with alignas pack expansion.
* error.c (dump_expr): Handle TREE_LIST.
* parser.c (cp_parser_std_attribute_list): Return error_mark if
make_pack_expansion returns an error.
* g++.dg/cpp0x/alignas11.C: New test.
diff --git gcc/cp/error.c gcc/cp/error.c
index d8c5d5e..10af9d0 100644
--- gcc/cp/error.c
+++ gcc/cp/error.c
@@ -2821,6 +2821,10 @@ dump_expr (cxx_pretty_printer *pp, tree t, int flags)
pp_string (pp, M_("*this"));
break;
+case TREE_LIST:
+ dump_expr_list (pp, t, flags);
+ break;
+
/* This list is incomplete, but should suffice for now.
It is very important that `sorry' does not call
`report_error_function'. That could cause an infinite loop. */
diff --git gcc/cp/parser.c gcc/cp/parser.c
index c1b6496..efca2e1 100644
--- gcc/cp/parser.c
+++ gcc/cp/parser.c
@@ -24842,8 +24842,12 @@ cp_parser_std_attribute_list (cp_parser *parser, tree
attr_ns)
error_at (token->location,
"expected attribute before %<...%>");
else
- TREE_VALUE (attribute)
- = make_pack_expansion (TREE_VALUE (attribute));
+ {
+ tree pack = make_pack_expansion (TREE_VALUE (attribute));
+ if (pack == error_mark_node)
+ return error_mark_node;
+ TREE_VALUE (attribute) = pack;
+ }
token = cp_lexer_peek_token (parser->lexer);
}
if (token->type != CPP_COMMA)
diff --git gcc/testsuite/g++.dg/cpp0x/alignas11.C
gcc/testsuite/g++.dg/cpp0x/alignas11.C
index e69de29..73c54da 100644
--- gcc/testsuite/g++.dg/cpp0x/alignas11.C
+++ gcc/testsuite/g++.dg/cpp0x/alignas11.C
@@ -0,0 +1,10 @@
+// PR c++/80241
+// { dg-do compile { target c++11 } }
+
+template
+struct A
+{
+ [[gnu::aligned (alignof(A))...]] char c; // { dg-error "expansion pattern" }
+};
+
+A a;
Marek
Re: [PATCH, rs6000] Fix PR target/80246, DFP builtins using the wrong types
On 3/30/17 12:15 PM, Segher Boessenkool wrote:
+/* { dg-final { scan-assembler-not "drintn\\." } } */
+/* { dg-final { scan-assembler-not "drintnq\\." } } */
+/* { dg-final { scan-assembler-not "dctfix" } } */
+/* { dg-final { scan-assembler-not "dctfixq" } } */
>>>
>>> If there is no "dctfix" there surely is no "dctfixq" either (i.e., your
>>> regexen aren't very tight).
>>
>> Ahh, true. I suppose I could also just look for "drintn" too,
>> since that would catch both drintn. and drintnq., ok with that
>> change?
>
> Please add a comment what instructions each regex is supposed to match, then?
> Okay with such a change.
Actually, the following is probably better. I'll go with this unless
you object.
/* { dg-final { scan-assembler-not "drintn\[q\]\." } } */
/* { dg-final { scan-assembler-not "dctfix\[q\]" } } */
/* { dg-final { scan-assembler-not "dcffix\[q\]" } } */
Peter
Re: [RFC] [PATCH v3 0/8] [i386] Use out-of-line stubs for ms_abi pro/epilogues
I have finally completed all tests for Cygwin and MinGW both 32- & 64-bit with no additional test failures. There are still 567 tests failing both pre- and post-patch with error "error while loading shared libraries: cyggfortran-4.dll: cannot open shared object file: No such file or directory" in all 32-bit tests even after my (fairly crude) patch to address that problem. So as a separate issue, I don't yet have a clean patch set to resolve the windows dll search path issue (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79867). I had to change the test program, as I was dependent upon XSI extensions which aren't available on Cygwin, so I'll need to repost that. Also, I had to make one small change in the "aligned SSE MOVs" patch, disabling it on SEH targets since gcc/config/i386/winnt.c does not currently support the REG_CFA_EXPRESSION note in its unwind emit. This optimization primarily targets 64-bit Wine anyway, where stack realignment is now required. Daniel
Re: [PATCH, rs6000] Fix PR target/80246, DFP builtins using the wrong types
> >> +/* { dg-final { scan-assembler-not "drintn\\." } } */
> >> +/* { dg-final { scan-assembler-not "drintnq\\." } } */
> >> +/* { dg-final { scan-assembler-not "dctfix" } } */
> >> +/* { dg-final { scan-assembler-not "dctfixq" } } */
> >
> > If there is no "dctfix" there surely is no "dctfixq" either (i.e., your
> > regexen aren't very tight).
>
> Ahh, true. I suppose I could also just look for "drintn" too,
> since that would catch both drintn. and drintnq., ok with that
> change?
Please add a comment what instructions each regex is supposed to match, then?
Okay with such a change.
Segher
Re: [C++ PATCH] Disable VLA initialization? (PR sanitizer/79993)
On 03/29/2017 01:31 PM, Jakub Jelinek wrote: Hi! GCC 4.8 and earlier didn't allow in C++ initialization of VLA and C doesn't allow it in any GCC release. This has changed when the support for C++1y VLA has been added, then reverted, but apparently only partially. The question is, do we want to support VLA initialization, if yes, with what exact semantics (in that case we need to fix up initialization from STRING_CST which is broken right now). The following patch is the variant that disables it (bootstrapped/regtested on x86_64-linux and i686-linux). Looking at what is emitted for initialization from non-STRING_CSTs, it seems that we consider UB if the VLA is shorter than the initializer array, and if it is longer, we somehow initialize the rest (dunno what exact C++ initialization kind, there are some testcases with ctors in the list below; for PODs zero initialization). So, if we want to keep it and just fix STRING_CST initialization, shall we emit memcpy from the array followed by whatever we emit right now for the excess elements (for initialization from STRING_CST, can it ever be anything but zero initialization? If not, then memset would do the job). I already chimed in on the bug report but just for the record, my view is that it would be preferable to continue to allow VLAs to be initialized as long as the excess initialization isn't allowed to trash the stack as it is now (bug 69517). The patch for that was committed into GCC 6 but had to be reverted due to interfering with Java's own implementation of exception). With Java removed I'd like to resubmit the patch in GCC 8. I don't think rejecting all VLA initialization just to avoid an Asan ICE with string literals is a good way to solve the problem. For one, it will break working programs that rely on what is currently accepted by GCC and for which it emits valid code. Longer term, users will likely come up with other ways to initialize their VLAs that may be even harder (or slower) for GCC to detect bugs in that the plain initialization. Martin
Re: [PATCH, rs6000] Fix PR target/80246, DFP builtins using the wrong types
On 3/29/17 6:29 PM, Peter Bergner wrote:
> On 3/29/17 5:28 PM, Segher Boessenkool wrote:
>>> +/* { dg-skip-if "" { powerpc*-*-darwin* } { "*" } { "" } } */
>>> +/* { dg-skip-if "" { powerpc*-*-*spe* } { "*" } { "" } } */
>>> +/* { dg-require-effective-target dfp } */
>>
>> You can leave off the default arguments "*" "". Do we really need to
>> explicitly skip on Darwin and SPE if there is a test for DFP anyway?
Ok, I removed the darwin and spe tests, since the dfp requirement will
disallow those. I made the same change to teh dfp-builtin-1.c test case
as well. I also failed to realize that dfp-builtin-1.c was failing due
to the check for no "stfd" and "lfd", which we will now expect to see,
since we're compiling the test case with -mcpu=power7, therefore I
updated the test case to expect them.
>>> +/* { dg-final { scan-assembler-not "drintn\\." } } */
>>> +/* { dg-final { scan-assembler-not "drintnq\\." } } */
>>> +/* { dg-final { scan-assembler-not "dctfix" } } */
>>> +/* { dg-final { scan-assembler-not "dctfixq" } } */
>>
>> If there is no "dctfix" there surely is no "dctfixq" either (i.e., your
>> regexen aren't very tight).
>
> Ahh, true. I suppose I could also just look for "drintn" too,
> since that would catch both drintn. and drintnq., ok with that
> change?
Fixed. Here is an updated patch. Is this better?
Peter
gcc/
PR target/80246
* config/rs6000/dfp.md (dfp_dxex_): Update mode of operand 0.
(dfp_diex_): Update mode of operand 1.
* doc/extend.texi (dxex, dxexq): Document change to return type.
(diex, diexq): Document change to argument type.
gcc/testsuite/
PR target/80246
* gcc.target/powerpc/dfp-builtin-1.c (dxex, dxexq): Update return types.
(diex, diexq): Update argument type.
* gcc.target/powerpc/pr80246.c: New test.
Index: gcc/config/rs6000/dfp.md
===
--- gcc/config/rs6000/dfp.md(revision 246539)
+++ gcc/config/rs6000/dfp.md(working copy)
@@ -348,9 +348,9 @@
[(set_attr "type" "dfp")])
(define_insn "dfp_dxex_"
- [(set (match_operand:D64_D128 0 "gpc_reg_operand" "=d")
- (unspec:D64_D128 [(match_operand:D64_D128 1 "gpc_reg_operand" "d")]
-UNSPEC_DXEX))]
+ [(set (match_operand:DI 0 "gpc_reg_operand" "=d")
+ (unspec:DI [(match_operand:D64_D128 1 "gpc_reg_operand" "d")]
+ UNSPEC_DXEX))]
"TARGET_DFP"
"dxex %0,%1"
[(set_attr "type" "dfp")])
@@ -357,7 +357,7 @@
(define_insn "dfp_diex_"
[(set (match_operand:D64_D128 0 "gpc_reg_operand" "=d")
- (unspec:D64_D128 [(match_operand:D64_D128 1 "gpc_reg_operand" "d")
+ (unspec:D64_D128 [(match_operand:DI 1 "gpc_reg_operand" "d")
(match_operand:D64_D128 2 "gpc_reg_operand" "d")]
UNSPEC_DXEX))]
"TARGET_DFP"
Index: gcc/doc/extend.texi
===
--- gcc/doc/extend.texi (revision 246539)
+++ gcc/doc/extend.texi (working copy)
@@ -15427,14 +15427,14 @@
of processors when hardware decimal floating point
(@option{-mhard-dfp}) is available:
@smallexample
-_Decimal64 __builtin_dxex (_Decimal64);
-_Decimal128 __builtin_dxexq (_Decimal128);
+long long __builtin_dxex (_Decimal64);
+long long __builtin_dxexq (_Decimal128);
_Decimal64 __builtin_ddedpd (int, _Decimal64);
_Decimal128 __builtin_ddedpdq (int, _Decimal128);
_Decimal64 __builtin_denbcd (int, _Decimal64);
_Decimal128 __builtin_denbcdq (int, _Decimal128);
-_Decimal64 __builtin_diex (_Decimal64, _Decimal64);
-_Decimal128 _builtin_diexq (_Decimal128, _Decimal128);
+_Decimal64 __builtin_diex (long long, _Decimal64);
+_Decimal128 _builtin_diexq (long long, _Decimal128);
_Decimal64 __builtin_dscli (_Decimal64, int);
_Decimal128 __builtin_dscliq (_Decimal128, int);
_Decimal64 __builtin_dscri (_Decimal64, int);
Index: gcc/testsuite/gcc.target/powerpc/dfp-builtin-1.c
===
--- gcc/testsuite/gcc.target/powerpc/dfp-builtin-1.c(revision 246539)
+++ gcc/testsuite/gcc.target/powerpc/dfp-builtin-1.c(working copy)
@@ -1,6 +1,4 @@
/* { dg-do compile { target { powerpc*-*-linux* } } } */
-/* { dg-skip-if "" { powerpc*-*-darwin* } { "*" } { "" } } */
-/* { dg-skip-if "" { powerpc*-*-*spe* } { "*" } { "" } } */
/* { dg-require-effective-target powerpc_vsx_ok } */
/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } {
"-mcpu=power7" } } */
/* { dg-options "-mcpu=power7 -O2" } */
@@ -10,11 +8,13 @@
/* { dg-final { scan-assembler-times "diex " 1} } */
/* { dg-final { scan-assembler-times "dscli " 2} } */
/* { dg-final { scan-assembler-times "dscri " 2} } */
+/* { dg-final { scan-assembler-times "std "1} } */
+/* { dg-final { scan-assembler-times "ld " 1} } */
+/* { dg-final { scan-assembler-times "stfd " 1} } */
+/* { dg-final { scan-assemb
Re: [PATCH PR80153]Always generate folded type conversion in tree-affine
On Thu, Mar 30, 2017 at 3:20 PM, Bin.Cheng wrote:
> On Thu, Mar 30, 2017 at 2:18 PM, Bin.Cheng wrote:
>> On Thu, Mar 30, 2017 at 1:44 PM, Richard Biener
>> wrote:
>>> On Thu, Mar 30, 2017 at 2:03 PM, Bin.Cheng wrote:
On Thu, Mar 30, 2017 at 11:37 AM, Richard Biener
wrote:
> On Wed, Mar 29, 2017 at 5:22 PM, Bin.Cheng wrote:
>> On Tue, Mar 28, 2017 at 1:34 PM, Richard Biener
>> wrote:
>>> On Tue, Mar 28, 2017 at 2:01 PM, Bin Cheng wrote:
Hi,
This patch is to fix PR80153. As analyzed in the PR, root cause is
tree_affine lacks
ability differentiating (unsigned)(ptr + offset) and (unsigned)ptr +
(unsigned)offset,
even worse, it always returns the former expression in
aff_combination_tree, which
is wrong if the original expression has the latter form. The patch
resolves the issue
by always returning the latter form expression, i.e, always trying to
generate folded
expression. Also as analyzed in comment, I think this change won't
result in substantial
code gen difference.
I also need to adjust get_computation_aff for test case
gcc.dg/tree-ssa/reassoc-19.c.
Well, I think the changed behavior is correct, but for case the
original pointer candidate
is chosen, it should be unnecessary to compute in uutype. Also this
adjustment only
generates (unsigned)(pointer + offset) which is generated by
tree-affine.c.
Bootstrap and test on x86_64 and AArch64. Is it OK?
>>>
>> Thanks for reviewing.
>>> Hmm. What is the desired goal? To have all elts added have
>>> comb->type as type? Then
>>> the type passed to add_elt_to_tree is redundant with comb->type. It
>>> looks like it
>>> is always passed comb->type now.
>> Yes, except pointer type comb->type, elts are converted to comb->type
>> with this patch.
>> The redundant type is removed in updated patch.
>>
>>>
>>> ISTR from past work in this area that it was important for pointer
>>> combinations to allow
>>> both pointer and sizetype elts at least.
>> Yes, It's still important to allow different types for pointer and
>> offset in pointer type comb.
>> I missed a pointer type check condition in the patch, fixed in updated
>> patch.
>>>
>>> Your change is incomplete I think, for the scale == -1 and
>>> POINTER_TYPE_P case
>>> elt is sizetype now, not of pointer type. As said above, we are
>>> trying to maintain
>>> both pointer and sizetype elts with like:
>>>
>>> if (scale == 1)
>>> {
>>> if (!expr)
>>> {
>>> if (POINTER_TYPE_P (TREE_TYPE (elt)))
>>> return elt;
>>> else
>>> return fold_convert (type1, elt);
>>> }
>>>
>>> where your earilier fold to type would result in not all cases handled
>>> the same
>>> (depending whether scale was -1 for example).
>> IIUC, it doesn't matter. For comb->type being pointer type, the
>> behavior remains the same.
>> For comb->type being unsigned T, this elt is converted to ptr_offtype,
>> rather than unsigned T,
>> this doesn't matter because ptr_offtype and unsigned T are equal to
>> each other, otherwise
>> tree_to_aff_combination shouldn't distribute it as a single elt.
>> Anyway, this is addressed in updated patch by checking pointer
>> comb->type additionally.
>> BTW, I think "scale==-1" case is a simple heuristic differentiating
>> pointer_base and offset.
>>
>>>
>>> Thus - shouldn't we simply drop the type argument (or rather the comb
>>> one?
>>> that wide_int_ext_for_comb looks weird given we get a widest_int as
>>> input
>>> and all the other wide_int_ext_for_comb calls around).
>>>
>>> And unconditionally convert to type, simplifying the rest of the code?
>> As said, for pointer type comb, we need to keep current behavior; for
>> other cases,
>> unconditionally convert to comb->type is the goal.
>>
>> Bootstrap and test on x86_64 and AArch64. Is this version OK?
>
> @@ -399,22 +400,20 @@ add_elt_to_tree (tree expr, tree type, tree elt,
> const widest_int &scale_in,
> if (POINTER_TYPE_P (TREE_TYPE (elt)))
> return elt;
> else
> - return fold_convert (type1, elt);
> + return fold_convert (type, elt);
> }
>
> the conversion should already have been done. For non-pointer comb->type
> it has been converted to type by your patch. For pointer-type comb->type
> it should be either pointer type or ptrofftype ('type') already as well.
>
> That said, can we do sth like
>
> @@ -384,6 +395,12 @@ add_elt_to_tree (tree expr, tree type
Re: [PATCH PR80153]Always generate folded type conversion in tree-affine
On Thu, Mar 30, 2017 at 2:18 PM, Bin.Cheng wrote:
> On Thu, Mar 30, 2017 at 1:44 PM, Richard Biener
> wrote:
>> On Thu, Mar 30, 2017 at 2:03 PM, Bin.Cheng wrote:
>>> On Thu, Mar 30, 2017 at 11:37 AM, Richard Biener
>>> wrote:
On Wed, Mar 29, 2017 at 5:22 PM, Bin.Cheng wrote:
> On Tue, Mar 28, 2017 at 1:34 PM, Richard Biener
> wrote:
>> On Tue, Mar 28, 2017 at 2:01 PM, Bin Cheng wrote:
>>> Hi,
>>> This patch is to fix PR80153. As analyzed in the PR, root cause is
>>> tree_affine lacks
>>> ability differentiating (unsigned)(ptr + offset) and (unsigned)ptr +
>>> (unsigned)offset,
>>> even worse, it always returns the former expression in
>>> aff_combination_tree, which
>>> is wrong if the original expression has the latter form. The patch
>>> resolves the issue
>>> by always returning the latter form expression, i.e, always trying to
>>> generate folded
>>> expression. Also as analyzed in comment, I think this change won't
>>> result in substantial
>>> code gen difference.
>>> I also need to adjust get_computation_aff for test case
>>> gcc.dg/tree-ssa/reassoc-19.c.
>>> Well, I think the changed behavior is correct, but for case the
>>> original pointer candidate
>>> is chosen, it should be unnecessary to compute in uutype. Also this
>>> adjustment only
>>> generates (unsigned)(pointer + offset) which is generated by
>>> tree-affine.c.
>>> Bootstrap and test on x86_64 and AArch64. Is it OK?
>>
> Thanks for reviewing.
>> Hmm. What is the desired goal? To have all elts added have
>> comb->type as type? Then
>> the type passed to add_elt_to_tree is redundant with comb->type. It
>> looks like it
>> is always passed comb->type now.
> Yes, except pointer type comb->type, elts are converted to comb->type
> with this patch.
> The redundant type is removed in updated patch.
>
>>
>> ISTR from past work in this area that it was important for pointer
>> combinations to allow
>> both pointer and sizetype elts at least.
> Yes, It's still important to allow different types for pointer and
> offset in pointer type comb.
> I missed a pointer type check condition in the patch, fixed in updated
> patch.
>>
>> Your change is incomplete I think, for the scale == -1 and
>> POINTER_TYPE_P case
>> elt is sizetype now, not of pointer type. As said above, we are
>> trying to maintain
>> both pointer and sizetype elts with like:
>>
>> if (scale == 1)
>> {
>> if (!expr)
>> {
>> if (POINTER_TYPE_P (TREE_TYPE (elt)))
>> return elt;
>> else
>> return fold_convert (type1, elt);
>> }
>>
>> where your earilier fold to type would result in not all cases handled
>> the same
>> (depending whether scale was -1 for example).
> IIUC, it doesn't matter. For comb->type being pointer type, the
> behavior remains the same.
> For comb->type being unsigned T, this elt is converted to ptr_offtype,
> rather than unsigned T,
> this doesn't matter because ptr_offtype and unsigned T are equal to
> each other, otherwise
> tree_to_aff_combination shouldn't distribute it as a single elt.
> Anyway, this is addressed in updated patch by checking pointer
> comb->type additionally.
> BTW, I think "scale==-1" case is a simple heuristic differentiating
> pointer_base and offset.
>
>>
>> Thus - shouldn't we simply drop the type argument (or rather the comb
>> one?
>> that wide_int_ext_for_comb looks weird given we get a widest_int as input
>> and all the other wide_int_ext_for_comb calls around).
>>
>> And unconditionally convert to type, simplifying the rest of the code?
> As said, for pointer type comb, we need to keep current behavior; for
> other cases,
> unconditionally convert to comb->type is the goal.
>
> Bootstrap and test on x86_64 and AArch64. Is this version OK?
@@ -399,22 +400,20 @@ add_elt_to_tree (tree expr, tree type, tree elt,
const widest_int &scale_in,
if (POINTER_TYPE_P (TREE_TYPE (elt)))
return elt;
else
- return fold_convert (type1, elt);
+ return fold_convert (type, elt);
}
the conversion should already have been done. For non-pointer comb->type
it has been converted to type by your patch. For pointer-type comb->type
it should be either pointer type or ptrofftype ('type') already as well.
That said, can we do sth like
@@ -384,6 +395,12 @@ add_elt_to_tree (tree expr, tree type, t
widest_int scale = wide_int_ext_for_comb (scale_in, comb);
+ if (! POINTER_TYPE_P (comb->type))
+elt = fold_convert (comb->type, elt);
>>
Re: [PATCH PR80153]Always generate folded type conversion in tree-affine
On Thu, Mar 30, 2017 at 1:44 PM, Richard Biener
wrote:
> On Thu, Mar 30, 2017 at 2:03 PM, Bin.Cheng wrote:
>> On Thu, Mar 30, 2017 at 11:37 AM, Richard Biener
>> wrote:
>>> On Wed, Mar 29, 2017 at 5:22 PM, Bin.Cheng wrote:
On Tue, Mar 28, 2017 at 1:34 PM, Richard Biener
wrote:
> On Tue, Mar 28, 2017 at 2:01 PM, Bin Cheng wrote:
>> Hi,
>> This patch is to fix PR80153. As analyzed in the PR, root cause is
>> tree_affine lacks
>> ability differentiating (unsigned)(ptr + offset) and (unsigned)ptr +
>> (unsigned)offset,
>> even worse, it always returns the former expression in
>> aff_combination_tree, which
>> is wrong if the original expression has the latter form. The patch
>> resolves the issue
>> by always returning the latter form expression, i.e, always trying to
>> generate folded
>> expression. Also as analyzed in comment, I think this change won't
>> result in substantial
>> code gen difference.
>> I also need to adjust get_computation_aff for test case
>> gcc.dg/tree-ssa/reassoc-19.c.
>> Well, I think the changed behavior is correct, but for case the original
>> pointer candidate
>> is chosen, it should be unnecessary to compute in uutype. Also this
>> adjustment only
>> generates (unsigned)(pointer + offset) which is generated by
>> tree-affine.c.
>> Bootstrap and test on x86_64 and AArch64. Is it OK?
>
Thanks for reviewing.
> Hmm. What is the desired goal? To have all elts added have
> comb->type as type? Then
> the type passed to add_elt_to_tree is redundant with comb->type. It
> looks like it
> is always passed comb->type now.
Yes, except pointer type comb->type, elts are converted to comb->type
with this patch.
The redundant type is removed in updated patch.
>
> ISTR from past work in this area that it was important for pointer
> combinations to allow
> both pointer and sizetype elts at least.
Yes, It's still important to allow different types for pointer and
offset in pointer type comb.
I missed a pointer type check condition in the patch, fixed in updated
patch.
>
> Your change is incomplete I think, for the scale == -1 and POINTER_TYPE_P
> case
> elt is sizetype now, not of pointer type. As said above, we are
> trying to maintain
> both pointer and sizetype elts with like:
>
> if (scale == 1)
> {
> if (!expr)
> {
> if (POINTER_TYPE_P (TREE_TYPE (elt)))
> return elt;
> else
> return fold_convert (type1, elt);
> }
>
> where your earilier fold to type would result in not all cases handled
> the same
> (depending whether scale was -1 for example).
IIUC, it doesn't matter. For comb->type being pointer type, the
behavior remains the same.
For comb->type being unsigned T, this elt is converted to ptr_offtype,
rather than unsigned T,
this doesn't matter because ptr_offtype and unsigned T are equal to
each other, otherwise
tree_to_aff_combination shouldn't distribute it as a single elt.
Anyway, this is addressed in updated patch by checking pointer
comb->type additionally.
BTW, I think "scale==-1" case is a simple heuristic differentiating
pointer_base and offset.
>
> Thus - shouldn't we simply drop the type argument (or rather the comb one?
> that wide_int_ext_for_comb looks weird given we get a widest_int as input
> and all the other wide_int_ext_for_comb calls around).
>
> And unconditionally convert to type, simplifying the rest of the code?
As said, for pointer type comb, we need to keep current behavior; for
other cases,
unconditionally convert to comb->type is the goal.
Bootstrap and test on x86_64 and AArch64. Is this version OK?
>>>
>>> @@ -399,22 +400,20 @@ add_elt_to_tree (tree expr, tree type, tree elt,
>>> const widest_int &scale_in,
>>> if (POINTER_TYPE_P (TREE_TYPE (elt)))
>>> return elt;
>>> else
>>> - return fold_convert (type1, elt);
>>> + return fold_convert (type, elt);
>>> }
>>>
>>> the conversion should already have been done. For non-pointer comb->type
>>> it has been converted to type by your patch. For pointer-type comb->type
>>> it should be either pointer type or ptrofftype ('type') already as well.
>>>
>>> That said, can we do sth like
>>>
>>> @@ -384,6 +395,12 @@ add_elt_to_tree (tree expr, tree type, t
>>>
>>>widest_int scale = wide_int_ext_for_comb (scale_in, comb);
>>>
>>> + if (! POINTER_TYPE_P (comb->type))
>>> +elt = fold_convert (comb->type, elt);
>>> + else
>>> +gcc_assert (POINTER_TYPE_P (TREE_TYPE (elt))
>>> + || types_compatible_p (TREE_TYPE (elt), type1));
>> Hmm, this assert can be broken since we d
Re: [PATCH] Fix various avx512 extraction issues (PR target/80206)
Hi Jakub, On 30 Mar 00:36, Jakub Jelinek wrote: > Hi! > > As the testcase shows, we ICE with -mavx512f -ffloat-store, because > at -O0 during expansion the destination is MEM, and the corresponding dup > operand is some pseudo. There are *_mask patterns that have just > register_operand / =v for the desination and vector_move_operand / 0C > for the corresponding dup operand (but this doesn't apply when the > destination is MEM), and then *_maskm patterns, that have > memory_operand / =m and corresponding dup operand memory_operand / 0, > but also requires rtx_equal_p between them in the condition, so that > doesn't match either. > The expanders have weirdo: > if (MEM_P (operands[0]) && GET_CODE (operands[3]) == CONST_VECTOR) > operands[0] = force_reg (mode, operands[0]); > which can't really ever work, because the expander's caller expects > the output to be stored in the original operands[0], but that is not > where it stores it. Furthermore, force_reg makes no sense for the > output operand. > > The following patch should fix that, bootstrapped/regtested on x86_64-linux > and i686-linux, ok for trunk? Patch is OK for trunk. -- Thanks, K
Re: [PATCH] Real fix for AIX exception handling
On Thu, Mar 30, 2017 at 4:11 AM, Michael Haubenwallner wrote: > On 03/29/2017 10:21 PM, David Edelsohn wrote: >> On Wed, Mar 29, 2017 at 3:50 PM, Jeff Law wrote: >>> On 03/27/2017 09:41 AM, David Edelsohn wrote: > > As far as I have discovered, the real problem with AIX exception handling > is > that the exception landing pads are symbols that must not (but still are) > exported from shared libraries - even libstdc++. > > I'm wondering if attached libtool(!)-patch would fix even that GDB > problem > once applied to each(!) shared library creation procedure. > > However, each workaround still applies as long as there's a single shared > library involved that has not stopped exporting these symbols yet. > > Thoughts? > > Maybe gcc's collect2 should apply this additional symbol filter itself > calling (AIX) ld, rather than leaving this to each build system? > > * m4/libtool.m4 (_LT_LINKER_SHLIBS): On AIX, GNU g++ generates > _GLOBAL__ symbols as, amongst others, landing pads for C++ exceptions. > These symbols must not be exported from shared libraries, or exception > handling may break for applications with runtime linking enabled. Hi, Michael Thanks for the analysis. The problem with EH for GDB involves static linking, not runtime linking. >>> >>> That seems to be my understanding as well. >>> And there seems to be different behavior for GCC 4.8 and GCC 4.9. >>> >>> Could it perhaps be an IPA issue -- we know IPA can change symbol >>> scope/linkage in some cases. Maybe it's mucking things up. Is there more >>> detail in a thread elsewhere on this issue? >> >> The problem is GCC EH tables and static linking. libstdc++ and the >> main application are ending up with two separate copies of the tables >> to register EH frames. > > When statically linked, shouldn't collect2 add libstdc++'s EH frames to > the main executable's registration table again? > Or is libstdc++'s constructor called instead? > >> Static linking worked in GCC 4.8, but not in GCC 4.9. I have been >> trying to understand what changed and if GCC 4.8 worked by accident. > > Wild guess: > When (and how) did you disable runtime linking (-G) for libstdc++? > Maybe there's a side effect related to -bsymbolic when statically linking > a shared object. Yes, two hypotheses are: 1) The removal of -G AIX linker option that allowed runtime overriding of libstdc++ symbols and somehow allowed merging of symbols when linking statically. 2) Change in order of initialization from AIX default breadth first to force SVR4-like depth first. > >> Note that AIX does not install a separate libstdc++ static archive and >> instead statically links against the shared object. > > Note that libtool's --with-aix-soname=svr4 would behave different here... > >> libstdc++ >> apparently uses the EH table referenced when it was bound by collect2 >> and the application uses the one created when the executable is >> created -- the libgcc_eh.a solution doesn't work. Again, the question >> is why this apparently functioned correctly for GCC.4.8. >> >> There was a change to constructor order around that time and that may >> have affected where EH frames were recorded. > > Next wild guess: When libstdc++'s EH frames are registered calling > libstdc++'s constructor even when statically linked rather than being > added to main executable's table, both registered EH tables may overlap > each other - where attached patch might help... Thanks, David
Re: [PATCH PR80153]Always generate folded type conversion in tree-affine
On Thu, Mar 30, 2017 at 2:03 PM, Bin.Cheng wrote:
> On Thu, Mar 30, 2017 at 11:37 AM, Richard Biener
> wrote:
>> On Wed, Mar 29, 2017 at 5:22 PM, Bin.Cheng wrote:
>>> On Tue, Mar 28, 2017 at 1:34 PM, Richard Biener
>>> wrote:
On Tue, Mar 28, 2017 at 2:01 PM, Bin Cheng wrote:
> Hi,
> This patch is to fix PR80153. As analyzed in the PR, root cause is
> tree_affine lacks
> ability differentiating (unsigned)(ptr + offset) and (unsigned)ptr +
> (unsigned)offset,
> even worse, it always returns the former expression in
> aff_combination_tree, which
> is wrong if the original expression has the latter form. The patch
> resolves the issue
> by always returning the latter form expression, i.e, always trying to
> generate folded
> expression. Also as analyzed in comment, I think this change won't
> result in substantial
> code gen difference.
> I also need to adjust get_computation_aff for test case
> gcc.dg/tree-ssa/reassoc-19.c.
> Well, I think the changed behavior is correct, but for case the original
> pointer candidate
> is chosen, it should be unnecessary to compute in uutype. Also this
> adjustment only
> generates (unsigned)(pointer + offset) which is generated by
> tree-affine.c.
> Bootstrap and test on x86_64 and AArch64. Is it OK?
>>> Thanks for reviewing.
Hmm. What is the desired goal? To have all elts added have
comb->type as type? Then
the type passed to add_elt_to_tree is redundant with comb->type. It
looks like it
is always passed comb->type now.
>>> Yes, except pointer type comb->type, elts are converted to comb->type
>>> with this patch.
>>> The redundant type is removed in updated patch.
>>>
ISTR from past work in this area that it was important for pointer
combinations to allow
both pointer and sizetype elts at least.
>>> Yes, It's still important to allow different types for pointer and
>>> offset in pointer type comb.
>>> I missed a pointer type check condition in the patch, fixed in updated
>>> patch.
Your change is incomplete I think, for the scale == -1 and POINTER_TYPE_P
case
elt is sizetype now, not of pointer type. As said above, we are
trying to maintain
both pointer and sizetype elts with like:
if (scale == 1)
{
if (!expr)
{
if (POINTER_TYPE_P (TREE_TYPE (elt)))
return elt;
else
return fold_convert (type1, elt);
}
where your earilier fold to type would result in not all cases handled the
same
(depending whether scale was -1 for example).
>>> IIUC, it doesn't matter. For comb->type being pointer type, the
>>> behavior remains the same.
>>> For comb->type being unsigned T, this elt is converted to ptr_offtype,
>>> rather than unsigned T,
>>> this doesn't matter because ptr_offtype and unsigned T are equal to
>>> each other, otherwise
>>> tree_to_aff_combination shouldn't distribute it as a single elt.
>>> Anyway, this is addressed in updated patch by checking pointer
>>> comb->type additionally.
>>> BTW, I think "scale==-1" case is a simple heuristic differentiating
>>> pointer_base and offset.
>>>
Thus - shouldn't we simply drop the type argument (or rather the comb one?
that wide_int_ext_for_comb looks weird given we get a widest_int as input
and all the other wide_int_ext_for_comb calls around).
And unconditionally convert to type, simplifying the rest of the code?
>>> As said, for pointer type comb, we need to keep current behavior; for
>>> other cases,
>>> unconditionally convert to comb->type is the goal.
>>>
>>> Bootstrap and test on x86_64 and AArch64. Is this version OK?
>>
>> @@ -399,22 +400,20 @@ add_elt_to_tree (tree expr, tree type, tree elt,
>> const widest_int &scale_in,
>> if (POINTER_TYPE_P (TREE_TYPE (elt)))
>> return elt;
>> else
>> - return fold_convert (type1, elt);
>> + return fold_convert (type, elt);
>> }
>>
>> the conversion should already have been done. For non-pointer comb->type
>> it has been converted to type by your patch. For pointer-type comb->type
>> it should be either pointer type or ptrofftype ('type') already as well.
>>
>> That said, can we do sth like
>>
>> @@ -384,6 +395,12 @@ add_elt_to_tree (tree expr, tree type, t
>>
>>widest_int scale = wide_int_ext_for_comb (scale_in, comb);
>>
>> + if (! POINTER_TYPE_P (comb->type))
>> +elt = fold_convert (comb->type, elt);
>> + else
>> +gcc_assert (POINTER_TYPE_P (TREE_TYPE (elt))
>> + || types_compatible_p (TREE_TYPE (elt), type1));
> Hmm, this assert can be broken since we do STRIP_NOPS converting to
> aff_tree. It's not compatible for signed and unsigned integer types.
> Also, with this patch, we can even support elt of short type in a
> unsigne
Re: [PATCH PR80153]Always generate folded type conversion in tree-affine
On Thu, Mar 30, 2017 at 11:37 AM, Richard Biener
wrote:
> On Wed, Mar 29, 2017 at 5:22 PM, Bin.Cheng wrote:
>> On Tue, Mar 28, 2017 at 1:34 PM, Richard Biener
>> wrote:
>>> On Tue, Mar 28, 2017 at 2:01 PM, Bin Cheng wrote:
Hi,
This patch is to fix PR80153. As analyzed in the PR, root cause is
tree_affine lacks
ability differentiating (unsigned)(ptr + offset) and (unsigned)ptr +
(unsigned)offset,
even worse, it always returns the former expression in
aff_combination_tree, which
is wrong if the original expression has the latter form. The patch
resolves the issue
by always returning the latter form expression, i.e, always trying to
generate folded
expression. Also as analyzed in comment, I think this change won't result
in substantial
code gen difference.
I also need to adjust get_computation_aff for test case
gcc.dg/tree-ssa/reassoc-19.c.
Well, I think the changed behavior is correct, but for case the original
pointer candidate
is chosen, it should be unnecessary to compute in uutype. Also this
adjustment only
generates (unsigned)(pointer + offset) which is generated by tree-affine.c.
Bootstrap and test on x86_64 and AArch64. Is it OK?
>>>
>> Thanks for reviewing.
>>> Hmm. What is the desired goal? To have all elts added have
>>> comb->type as type? Then
>>> the type passed to add_elt_to_tree is redundant with comb->type. It
>>> looks like it
>>> is always passed comb->type now.
>> Yes, except pointer type comb->type, elts are converted to comb->type
>> with this patch.
>> The redundant type is removed in updated patch.
>>
>>>
>>> ISTR from past work in this area that it was important for pointer
>>> combinations to allow
>>> both pointer and sizetype elts at least.
>> Yes, It's still important to allow different types for pointer and
>> offset in pointer type comb.
>> I missed a pointer type check condition in the patch, fixed in updated patch.
>>>
>>> Your change is incomplete I think, for the scale == -1 and POINTER_TYPE_P
>>> case
>>> elt is sizetype now, not of pointer type. As said above, we are
>>> trying to maintain
>>> both pointer and sizetype elts with like:
>>>
>>> if (scale == 1)
>>> {
>>> if (!expr)
>>> {
>>> if (POINTER_TYPE_P (TREE_TYPE (elt)))
>>> return elt;
>>> else
>>> return fold_convert (type1, elt);
>>> }
>>>
>>> where your earilier fold to type would result in not all cases handled the
>>> same
>>> (depending whether scale was -1 for example).
>> IIUC, it doesn't matter. For comb->type being pointer type, the
>> behavior remains the same.
>> For comb->type being unsigned T, this elt is converted to ptr_offtype,
>> rather than unsigned T,
>> this doesn't matter because ptr_offtype and unsigned T are equal to
>> each other, otherwise
>> tree_to_aff_combination shouldn't distribute it as a single elt.
>> Anyway, this is addressed in updated patch by checking pointer
>> comb->type additionally.
>> BTW, I think "scale==-1" case is a simple heuristic differentiating
>> pointer_base and offset.
>>
>>>
>>> Thus - shouldn't we simply drop the type argument (or rather the comb one?
>>> that wide_int_ext_for_comb looks weird given we get a widest_int as input
>>> and all the other wide_int_ext_for_comb calls around).
>>>
>>> And unconditionally convert to type, simplifying the rest of the code?
>> As said, for pointer type comb, we need to keep current behavior; for
>> other cases,
>> unconditionally convert to comb->type is the goal.
>>
>> Bootstrap and test on x86_64 and AArch64. Is this version OK?
>
> @@ -399,22 +400,20 @@ add_elt_to_tree (tree expr, tree type, tree elt,
> const widest_int &scale_in,
> if (POINTER_TYPE_P (TREE_TYPE (elt)))
> return elt;
> else
> - return fold_convert (type1, elt);
> + return fold_convert (type, elt);
> }
>
> the conversion should already have been done. For non-pointer comb->type
> it has been converted to type by your patch. For pointer-type comb->type
> it should be either pointer type or ptrofftype ('type') already as well.
>
> That said, can we do sth like
>
> @@ -384,6 +395,12 @@ add_elt_to_tree (tree expr, tree type, t
>
>widest_int scale = wide_int_ext_for_comb (scale_in, comb);
>
> + if (! POINTER_TYPE_P (comb->type))
> +elt = fold_convert (comb->type, elt);
> + else
> +gcc_assert (POINTER_TYPE_P (TREE_TYPE (elt))
> + || types_compatible_p (TREE_TYPE (elt), type1));
Hmm, this assert can be broken since we do STRIP_NOPS converting to
aff_tree. It's not compatible for signed and unsigned integer types.
Also, with this patch, we can even support elt of short type in a
unsigned long comb, though this is useless.
> +
>if (scale == -1
>&& POINTER_TYPE_P (TREE_TYPE (elt)))
> {
>
> that is clearly do the conversion at the start in a way the state
Re: [PR 77333] Fix fntypes of calls calling clones
On Wed, 29 Mar 2017, Martin Jambor wrote: > Hi, > > On Thu, Mar 16, 2017 at 05:57:51PM +0100, Martin Jambor wrote: > > Hi, > > > > On Mon, Mar 13, 2017 at 01:46:47PM +0100, Richard Biener wrote: > > > On Fri, 10 Mar 2017, Martin Jambor wrote: > > > > > > > Hi, > > > > > > > > PR 77333 is a i686-windows target bug, which however has its root in > > > > our general mechanism of adjusting gimple statements when redirecting > > > > call graph edge. Basically, these three things trigger it: > > > > > > > > 1) IPA-CP figures out that the this parameter of a C++ class method is > > > >unused and because the class is in an anonymous namespace, it can > > > >be removed and all calls adjusted. That effectively changes a > > > >normal method into a static method and so internally, its type > > > >changes from METHOD_TYPE to FUNCTION_TYPE. > > > > > > > > 2) Since the fix of PR 57330, we do not update gimple_call_fntype to > > > >match the new type, in fact we explicitely set it to the old, now > > > >invalid, type (see redirect_call_stmt_to_callee in cgraph.c). > > > > > > > > 3) Function ix86_get_callcvt which decides on call ABI, ends with the > > > >following condition: > > > > > > > > if (ret != 0 > > > > || is_stdarg > > > > || TREE_CODE (type) != METHOD_TYPE > > > > || ix86_function_type_abi (type) != MS_ABI) > > > >return IX86_CALLCVT_CDECL | ret; > > > > > > > > return IX86_CALLCVT_THISCALL; > > > > > > > >...and since now the callee is no longer a METHOD_TYPE but callers > > > >still think that they are, leading to calling convention mismatches > > > >and subsequent crashes. It took me quite a lot of time to come up > > > >with a small testcase (reproducible using wine) but eventually I > > > >managed. > > > > > > > > The fix is not to do 2) above, but doing so without re-introducing PR > > > > 57330, of course. > > ... > > > > > > > In general I am sympathetic with not doing any IPA propagation > > > across call stmt signature incompatibilties. Of course we may > > > be still too strict in those compatibility check... > > > > > > > So the alternative would be to re-check when doing the gimple > > > > statement adjustment and if the types match, then set the correct new > > > > gimple_fntype and if they don't... then we can either leave it be or > > > > just run the same type transformation on it as we did on the callee, > > > > though they would be bogus either way. That is implemented in the > > > > attached patch. > > > > > ... > > > After talking to Honza today, we decided to probably go this route and > > use the patch doing the type conversion at acall-sites when necessary. > > Honza promised to review the patch soon (he wants to figure out why > > former_clone_of can be NULL, something I decided not to bother about > > since at that time I thought the other approach was going to be > > preferable). > > > > and this is a slightly adjusted patch that is a result of what we > talked about. I know that it is potentially disruptive change, so I > have tested it with: > - bootstrap and testing and LTO-bootstrap and testing on x86_64-linux, > - bootstrap and testing on i686-linux, ppc64le-linux and ia64-linux > - bootstrap on aarch64-linux (no testing because there is no dejagnu > installed on gcc117.fsffrance.org), > - testing on i686-w64-mingw32 on Linux+wine, and > - testing on powerpc-aix is underway. > > OK for trunk (and subsequently to backport to gcc 6 and 5)? Ok. Ok to backport after some burn-in. Richard. > Thanks, > > Martin > > 2017-03-24 Martin Jambor > > PR ipa/77333 > * cgraph.h (cgraph_build_function_type_skip_args): Declare. > * cgraph.c (redirect_call_stmt_to_callee): Set gimple fntype so that > it reflects the signature changes performed at the callee side. > * cgraphclones.c (build_function_type_skip_args): Make public, renamed > to cgraph_build_function_type_skip_args. > (build_function_decl_skip_args): Adjust call to the above function. > > testsuite/ > * g++.dg/ipa/pr77333.C: New test. > --- > gcc/cgraph.c | 17 +- > gcc/cgraph.h | 2 ++ > gcc/cgraphclones.c | 9 +++--- > gcc/testsuite/g++.dg/ipa/pr77333.C | 65 > ++ > 4 files changed, 88 insertions(+), 5 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/ipa/pr77333.C > > diff --git a/gcc/cgraph.c b/gcc/cgraph.c > index 839388496ee..92ae0910c60 100644 > --- a/gcc/cgraph.c > +++ b/gcc/cgraph.c > @@ -1424,8 +1424,23 @@ cgraph_edge::redirect_call_stmt_to_callee (void) >if (skip_bounds) > new_stmt = chkp_copy_call_skip_bounds (new_stmt); > > + tree old_fntype = gimple_call_fntype (e->call_stmt); >gimple_call_set_fndecl (new_stmt, e->callee->decl); > - gimple_call_set_fntype (new_stmt, gimple_call_fntype
Re: [PATCH, GCC/testsuite/ARM, stage4] Compile atomic_loaddi_11 for Cortex-R5
Ping?
Best regards,
Thomas
On 23/03/17 17:09, Thomas Preudhomme wrote:
My apologize, this works for both -march of -mcpu not cortex-r4 in RUNTESTFLAGS.
ChangeLog entry is unchanged:
*** gcc/testsuite/ChangeLog ***
2017-03-22 Thomas Preud'homme
Sorry, I forgot about -march. Hold on.
On 23/03/17 16:51, Thomas Preudhomme wrote:
Please find attached an updated patch. ChangeLog entry unchanged:
*** gcc/testsuite/ChangeLog ***
2017-03-22 Thomas Preud'homme
Mmmh I probably need to add a dg-skip-if in there. Will respin the patch.
Best regards,
Thomas
On 23/03/17 16:10, Richard Earnshaw (lists) wrote:
On 23/03/17 16:02, Thomas Preudhomme wrote:
Hi,
gcc.target/arm/atomic_loaddi_11.c testcase contributed in r246365 does
not test the changed code since ARMv7-R does not have division
instructions in ARM state. This patch changes it to target Cortex-R5
processor instead which does have division instructions in ARM state.
ChangeLog entry is as follows:
*** gcc/testsuite/ChangeLog ***
2017-03-22 Thomas Preud'homme
Will that work properly if doing multilib testing with a specific CPU
target?
R.
diff --git a/gcc/testsuite/gcc.target/arm/atomic_loaddi_11.c b/gcc/testsuite/gcc.target/arm/atomic_loaddi_11.c
index 275669bd76356dc7c7b6a5373792d9a5089ede51..4ada2efd5f047154f2ca2fb39e9432c96ee1d42b 100644
--- a/gcc/testsuite/gcc.target/arm/atomic_loaddi_11.c
+++ b/gcc/testsuite/gcc.target/arm/atomic_loaddi_11.c
@@ -1,7 +1,6 @@
/* { dg-do compile } */
/* { dg-require-effective-target arm_arch_v7r_ok } */
-/* { dg-options "-O2" } */
-/* { dg-add-options arm_arch_v7r } */
+/* { dg-options "-O2 -mcpu=cortex-r5" } */
#include
Re: [PATCH, GCC/ARM, gcc-6-branch] Fix PR80082: LDRD erronously used for 64bit load on ARMv7-R
Ping?
Best regards,
Thomas
On 27/03/17 12:15, Thomas Preudhomme wrote:
Hi,
Currently GCC is happy to use LDRD to perform a 64bit load on ARMv7-R,
as shown by the testcase on this patch. However, LDRD is only atomic
when LPAE extensions is available, which they are not for ARMv7-R. This
commit solve the issue by introducing a new feature bit to distinguish
LPAE extensions instead of deducing it from div instruction
availability.
ChangeLog entries are as follow:
*** gcc/ChangeLog ***
2017-03-22 Thomas Preud'homme
PR target/80082
* config/arm/arm-protos.h (FL_LPAE): Define macro.
(FL_FOR_ARCH7VE): Add FL_LPAE.
(arm_arch_lpae): Declare extern.
* config/arm/arm.c (arm_arch_lpae): Declare.
(arm_option_override): Define arm_arch_lpae.
* config/arm/arm.h (TARGET_HAVE_LPAE): Redefine in term of
arm_arch_lpae.
*** gcc/testsuite/ChangeLog ***
2017-03-22 Thomas Preud'homme
PR target/80082
* gcc.target/arm/atomic_loaddi_10.c: New testcase.
* gcc.target/arm/atomic_loaddi_11.c: Likewise.
Testing: bootstrapped for -march=armv7ve and testsuite shows no regression.
Is this ok for gcc-6-branch?
Best regards,
Thomas
diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h
index 0083673b161a49e19388c72d3a413aeb481dbfa3..dea00e42551c8295f7e83a72ddb81ae8e9c8e02d 100644
--- a/gcc/config/arm/arm-protos.h
+++ b/gcc/config/arm/arm-protos.h
@@ -360,7 +360,7 @@ extern bool arm_is_constant_pool_ref (rtx);
#define FL_STRONG (1 << 8) /* StrongARM */
#define FL_ARCH5E (1 << 9)/* DSP extensions to v5 */
#define FL_XSCALE (1 << 10) /* XScale */
-/* spare (1 << 11) */
+#define FL_LPAE (1 << 11) /* ARMv7-A LPAE. */
#define FL_ARCH6 (1 << 12) /* Architecture rel 6. Adds
media instructions. */
#define FL_VFPV2 (1 << 13) /* Vector Floating Point V2. */
@@ -412,7 +412,7 @@ extern bool arm_is_constant_pool_ref (rtx);
#define FL_FOR_ARCH6M (FL_FOR_ARCH6 & ~FL_NOTM)
#define FL_FOR_ARCH7 ((FL_FOR_ARCH6T2 & ~FL_NOTM) | FL_ARCH7)
#define FL_FOR_ARCH7A (FL_FOR_ARCH7 | FL_NOTM | FL_ARCH6K)
-#define FL_FOR_ARCH7VE (FL_FOR_ARCH7A | FL_THUMB_DIV | FL_ARM_DIV)
+#define FL_FOR_ARCH7VE (FL_FOR_ARCH7A | FL_THUMB_DIV | FL_ARM_DIV | FL_LPAE)
#define FL_FOR_ARCH7R (FL_FOR_ARCH7A | FL_THUMB_DIV)
#define FL_FOR_ARCH7M (FL_FOR_ARCH7 | FL_THUMB_DIV)
#define FL_FOR_ARCH7EM (FL_FOR_ARCH7M | FL_ARCH7EM)
@@ -608,6 +608,9 @@ extern int arm_arch_thumb2;
extern int arm_arch_arm_hwdiv;
extern int arm_arch_thumb_hwdiv;
+/* Nonzero if this chip supports the Large Physical Address Extension. */
+extern int arm_arch_lpae;
+
/* Nonzero if chip disallows volatile memory access in IT block. */
extern int arm_arch_no_volatile_ce;
diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index ad123dde991a3e4c4b9563ee6ebb84981767988f..e93ff7f7d8583b653570cbb8605df5a10bfcc6f4 100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -254,8 +254,7 @@ extern void (*arm_lang_output_object_attributes_hook)(void);
#define TARGET_HAVE_LDREX((arm_arch6 && TARGET_ARM) || arm_arch7)
/* Nonzero if this chip supports LPAE. */
-#define TARGET_HAVE_LPAE \
- (arm_arch7 && ARM_FSET_HAS_CPU1 (insn_flags, FL_FOR_ARCH7VE))
+#define TARGET_HAVE_LPAE (arm_arch_lpae)
/* Nonzero if this chip supports ldrex{bh} and strex{bh}. */
#define TARGET_HAVE_LDREXBH ((arm_arch6k && TARGET_ARM) || arm_arch7)
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index c3c89b866355708d91dd2a3dab1e4b33f2215ff8..44bfb53a288f57fbc43a0bd146e193b768d939d2 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -859,6 +859,9 @@ int arm_arch_thumb2;
int arm_arch_arm_hwdiv;
int arm_arch_thumb_hwdiv;
+/* Nonzero if this chip supports the Large Physical Address Extension. */
+int arm_arch_lpae;
+
/* Nonzero if chip disallows volatile memory access in IT block. */
int arm_arch_no_volatile_ce;
@@ -3181,6 +3184,7 @@ arm_option_override (void)
arm_arch_iwmmxt2 = ARM_FSET_HAS_CPU1 (insn_flags, FL_IWMMXT2);
arm_arch_thumb_hwdiv = ARM_FSET_HAS_CPU1 (insn_flags, FL_THUMB_DIV);
arm_arch_arm_hwdiv = ARM_FSET_HAS_CPU1 (insn_flags, FL_ARM_DIV);
+ arm_arch_lpae = ARM_FSET_HAS_CPU1 (insn_flags, FL_LPAE);
arm_arch_no_volatile_ce = ARM_FSET_HAS_CPU1 (insn_flags, FL_NO_VOLATILE_CE);
arm_tune_cortex_a9 = (arm_tune == cortexa9) != 0;
arm_arch_crc = ARM_FSET_HAS_CPU1 (insn_flags, FL_CRC32);
diff --git a/gcc/testsuite/gcc.target/arm/atomic_loaddi_10.c b/gcc/testsuite/gcc.target/arm/atomic_loaddi_10.c
new file mode 100644
index ..ecc3d06d0c9f5966daa3ce7e2d52e09d14e0cbc8
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/atomic_loaddi_10.c
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target arm_arch_v7ve_ok } */
+/* { dg-options "-O2" } */
+/* { dg-add-options arm_arch_v7ve } */
+
+#include
+
+atomic_llong x = 0;
+
+atomic_llong get_x()
+{
Re: [PATCH PR80153]Always generate folded type conversion in tree-affine
On Wed, Mar 29, 2017 at 5:22 PM, Bin.Cheng wrote:
> On Tue, Mar 28, 2017 at 1:34 PM, Richard Biener
> wrote:
>> On Tue, Mar 28, 2017 at 2:01 PM, Bin Cheng wrote:
>>> Hi,
>>> This patch is to fix PR80153. As analyzed in the PR, root cause is
>>> tree_affine lacks
>>> ability differentiating (unsigned)(ptr + offset) and (unsigned)ptr +
>>> (unsigned)offset,
>>> even worse, it always returns the former expression in
>>> aff_combination_tree, which
>>> is wrong if the original expression has the latter form. The patch
>>> resolves the issue
>>> by always returning the latter form expression, i.e, always trying to
>>> generate folded
>>> expression. Also as analyzed in comment, I think this change won't result
>>> in substantial
>>> code gen difference.
>>> I also need to adjust get_computation_aff for test case
>>> gcc.dg/tree-ssa/reassoc-19.c.
>>> Well, I think the changed behavior is correct, but for case the original
>>> pointer candidate
>>> is chosen, it should be unnecessary to compute in uutype. Also this
>>> adjustment only
>>> generates (unsigned)(pointer + offset) which is generated by tree-affine.c.
>>> Bootstrap and test on x86_64 and AArch64. Is it OK?
>>
> Thanks for reviewing.
>> Hmm. What is the desired goal? To have all elts added have
>> comb->type as type? Then
>> the type passed to add_elt_to_tree is redundant with comb->type. It
>> looks like it
>> is always passed comb->type now.
> Yes, except pointer type comb->type, elts are converted to comb->type
> with this patch.
> The redundant type is removed in updated patch.
>
>>
>> ISTR from past work in this area that it was important for pointer
>> combinations to allow
>> both pointer and sizetype elts at least.
> Yes, It's still important to allow different types for pointer and
> offset in pointer type comb.
> I missed a pointer type check condition in the patch, fixed in updated patch.
>>
>> Your change is incomplete I think, for the scale == -1 and POINTER_TYPE_P
>> case
>> elt is sizetype now, not of pointer type. As said above, we are
>> trying to maintain
>> both pointer and sizetype elts with like:
>>
>> if (scale == 1)
>> {
>> if (!expr)
>> {
>> if (POINTER_TYPE_P (TREE_TYPE (elt)))
>> return elt;
>> else
>> return fold_convert (type1, elt);
>> }
>>
>> where your earilier fold to type would result in not all cases handled the
>> same
>> (depending whether scale was -1 for example).
> IIUC, it doesn't matter. For comb->type being pointer type, the
> behavior remains the same.
> For comb->type being unsigned T, this elt is converted to ptr_offtype,
> rather than unsigned T,
> this doesn't matter because ptr_offtype and unsigned T are equal to
> each other, otherwise
> tree_to_aff_combination shouldn't distribute it as a single elt.
> Anyway, this is addressed in updated patch by checking pointer
> comb->type additionally.
> BTW, I think "scale==-1" case is a simple heuristic differentiating
> pointer_base and offset.
>
>>
>> Thus - shouldn't we simply drop the type argument (or rather the comb one?
>> that wide_int_ext_for_comb looks weird given we get a widest_int as input
>> and all the other wide_int_ext_for_comb calls around).
>>
>> And unconditionally convert to type, simplifying the rest of the code?
> As said, for pointer type comb, we need to keep current behavior; for
> other cases,
> unconditionally convert to comb->type is the goal.
>
> Bootstrap and test on x86_64 and AArch64. Is this version OK?
@@ -399,22 +400,20 @@ add_elt_to_tree (tree expr, tree type, tree elt,
const widest_int &scale_in,
if (POINTER_TYPE_P (TREE_TYPE (elt)))
return elt;
else
- return fold_convert (type1, elt);
+ return fold_convert (type, elt);
}
the conversion should already have been done. For non-pointer comb->type
it has been converted to type by your patch. For pointer-type comb->type
it should be either pointer type or ptrofftype ('type') already as well.
That said, can we do sth like
@@ -384,6 +395,12 @@ add_elt_to_tree (tree expr, tree type, t
widest_int scale = wide_int_ext_for_comb (scale_in, comb);
+ if (! POINTER_TYPE_P (comb->type))
+elt = fold_convert (comb->type, elt);
+ else
+gcc_assert (POINTER_TYPE_P (TREE_TYPE (elt))
+ || types_compatible_p (TREE_TYPE (elt), type1));
+
if (scale == -1
&& POINTER_TYPE_P (TREE_TYPE (elt)))
{
that is clearly do the conversion at the start in a way the state
of elt is more clear?
Richard.
> Thanks,
> bin
>
> 2017-03-28 Bin Cheng
>
> PR tree-optimization/80153
> * tree-affine.c (add_elt_to_tree): Remove parameter TYPE. Use type
> of parameter COMB. Convert elt to type of COMB it COMB is not of
> pointer type.
> (aff_combination_to_tree): Update calls to add_elt_to_tree.
> * tree-ssa-loop-ivopts.c (alloc_iv): Pass in consistent types.
> (get_computation_aff): Use
Re: [PATCH] Real fix for AIX exception handling
On 03/29/2017 10:21 PM, David Edelsohn wrote:
> On Wed, Mar 29, 2017 at 3:50 PM, Jeff Law wrote:
>> On 03/27/2017 09:41 AM, David Edelsohn wrote:
As far as I have discovered, the real problem with AIX exception handling
is
that the exception landing pads are symbols that must not (but still are)
exported from shared libraries - even libstdc++.
I'm wondering if attached libtool(!)-patch would fix even that GDB
problem
once applied to each(!) shared library creation procedure.
However, each workaround still applies as long as there's a single shared
library involved that has not stopped exporting these symbols yet.
Thoughts?
Maybe gcc's collect2 should apply this additional symbol filter itself
calling (AIX) ld, rather than leaving this to each build system?
* m4/libtool.m4 (_LT_LINKER_SHLIBS): On AIX, GNU g++ generates
_GLOBAL__ symbols as, amongst others, landing pads for C++ exceptions.
These symbols must not be exported from shared libraries, or exception
handling may break for applications with runtime linking enabled.
>>>
>>>
>>> Hi, Michael
>>>
>>> Thanks for the analysis.
>>>
>>> The problem with EH for GDB involves static linking, not runtime
>>> linking.
>>
>> That seems to be my understanding as well.
>>
>>> And there seems to be different behavior for GCC 4.8 and GCC
>>> 4.9.
>>
>> Could it perhaps be an IPA issue -- we know IPA can change symbol
>> scope/linkage in some cases. Maybe it's mucking things up. Is there more
>> detail in a thread elsewhere on this issue?
>
> The problem is GCC EH tables and static linking. libstdc++ and the
> main application are ending up with two separate copies of the tables
> to register EH frames.
When statically linked, shouldn't collect2 add libstdc++'s EH frames to
the main executable's registration table again?
Or is libstdc++'s constructor called instead?
> Static linking worked in GCC 4.8, but not in GCC 4.9. I have been
> trying to understand what changed and if GCC 4.8 worked by accident.
Wild guess:
When (and how) did you disable runtime linking (-G) for libstdc++?
Maybe there's a side effect related to -bsymbolic when statically linking
a shared object.
> Note that AIX does not install a separate libstdc++ static archive and
> instead statically links against the shared object.
Note that libtool's --with-aix-soname=svr4 would behave different here...
> libstdc++
> apparently uses the EH table referenced when it was bound by collect2
> and the application uses the one created when the executable is
> created -- the libgcc_eh.a solution doesn't work. Again, the question
> is why this apparently functioned correctly for GCC.4.8.
>
> There was a change to constructor order around that time and that may
> have affected where EH frames were recorded.
Next wild guess: When libstdc++'s EH frames are registered calling
libstdc++'s constructor even when statically linked rather than being
added to main executable's table, both registered EH tables may overlap
each other - where attached patch might help...
Thanks!
/haubi/
>From 7ed6bd3bba4e3161e761d4cdb8393ec9bcf98038 Mon Sep 17 00:00:00 2001
From: Michael Haubenwallner
Date: Mon, 8 Feb 2016 12:37:56 +0100
Subject: [PATCH] libgcc: On AIX, increase chances to find landing pads for
exceptions.
* unwind-dw2-fde.c (_Unwind_Find_FDE): Stop assuming registered
object's address ranges to not overlap.
---
libgcc/ChangeLog| 6 ++
libgcc/unwind-dw2-fde.c | 20
2 files changed, 26 insertions(+)
diff --git a/libgcc/ChangeLog b/libgcc/ChangeLog
index 4bae69f..ffa6f60 100644
--- a/libgcc/ChangeLog
+++ b/libgcc/ChangeLog
@@ -1,3 +1,9 @@
+2016-02-08 Michael Haubenwallner
+
+ On AIX, increase chances to find landing pads for exceptions.
+ * unwind-dw2-fde.c (_Unwind_Find_FDE): Stop assuming registered
+ object's address ranges to not overlap.
+
2017-03-10 John Marino
* config/aarch64/freebsd-unwind.h: New file.
diff --git a/libgcc/unwind-dw2-fde.c b/libgcc/unwind-dw2-fde.c
index 02b588d..9cf2d65 100644
--- a/libgcc/unwind-dw2-fde.c
+++ b/libgcc/unwind-dw2-fde.c
@@ -1054,7 +1054,27 @@ _Unwind_Find_FDE (void *pc, struct dwarf_eh_bases *bases)
f = search_object (ob, pc);
if (f)
goto fini;
+ /* In an ideal world, even on AIX, we could break here because
+ objects would not overlap. But the larger an application is,
+ the more likely an "overlap" may happen (on AIX) because of:
+ - Shared libraries do export the FDE symbols ("_GLOBAL__F*"),
+ which is a bug in their build system, out of gcc's control.
+ - Other shared libraries, or the main executable, may contain
+ identical or similar object files - which is suboptimal, but
+ may be intentional. However, exporting their FDE symbols,
+ which may have identical symbol names as in their original
+ shared libraries, again is a bug in their build system, but
+ s
Re: [PATCH][RFC] Fix P1 PR77498
On Wed, 29 Mar 2017, Jeff Law wrote: > On 03/29/2017 04:05 AM, Richard Biener wrote: > > > > After quite some pondering over this and other related bugs I propose > > the following for GCC 7 which tames down PRE a bit (back to levels > > of GCC 6). Technically it's the wrong place to fix this, we do > > have measures in place during elimination but they are not in effect > > at -O2. For GCC 8 I'd like to be more aggressive there but that > > would require to enable predictive commoning at -O2 (with some > > limits to its unrolling) to not lose optimization opportunities. > > > > The other option is to ignore this issue and postpone the solution > > to GCC 8. > > > > Bootstrapped / tested on x86_64-unknown-linux-gnu. > > > > Any preference? > > > > Thanks, > > Richard. > > > > 2017-03-29 Richard Biener > > > > PR tree-optimization/77498 > > * tree-ssa-pre.c (phi_translate_1): Do not allow simplifications > > to non-constants over backedges. > > > > * gfortran.dg/pr77498.f: New testcase. > I've got a slight preference for this patch. > > If you had a good start on the real fix then I'd lean more towards postponing. I wouldn't it yet call "real fix" but just an idea (where I tested the PRE side already, with the expected testsuite regressions). I've done no benchmarking at all for that. For the proposed patch the situation is that we're only going to remove some PRE that was done additionally over GCC 6 because of the rev. that caused the regression, so I have confidence that it won't make things worse when comparing to GCC 6. Thus I've now applied the patch. Richard.
