Re: [PING 3][PATCH] track dynamic allocation in strlen (PR 91582)
On Fri, 2019-12-13 at 17:55 -0700, Martin Sebor wrote: > After more testing by Jeff's buildbot and correcting the problems > it exposed I have committed the attached patch in r279392. And just to close the loop on this. Your last version fixed all the issues I saw in the tester. jeff
Re: C++ PATCH for c++/88337 - Implement P1327R1: Allow dynamic_cast in constexpr
On Fri, Dec 13, 2019 at 05:56:57PM -0500, Jason Merrill wrote:
> On 12/13/19 3:20 PM, Marek Polacek wrote:
> > + /* Given dynamic_cast(v),
> > +
> > + [expr.dynamic.cast] If C is the class type to which T points or
> > refers,
> > + the runtime check logically executes as follows:
> > +
> > + If, in the most derived object pointed (referred) to by v, v points
> > + (refers) to a public base class subobject of a C object, and if only
> > + one object of type C is derived from the subobject pointed (referred)
> > + to by v the result points (refers) to that C object.
> > +
> > + In this case, HINT >= 0. */
> > + if (hint >= 0)
>
> Won't this code work for hint == -3 as well?
Yes, it does. In fact, none of the tests was testing the hint == -3 case, so
I've fixed the code up and added constexpr-dynamic15.C to test it.
> > +{
> > + /* Look for a component with type TYPE. */
> > + obj = get_component_with_type (obj, type);
>
> You don't seem to use mdtype at all in this case. Shouldn't
> get_component_with_type stop at mdtype if it hasn't found type yet?
It was used for diagnostics but not in get_component_with_type. It makes
sense to stop at MDTYPE; I've adjusted the code to do so. E.g., if
we have OBJ in the form of g.D.2121.D.2122.D.2123.D.2124, usually the
component with the most derived type is "g", but in a 'tor, it can be
a different component too.
> > + /* If not found or not accessible, give an error. */
> > + if (obj == NULL_TREE || obj == error_mark_node)
> > + {
> > + if (reference_p)
> > + {
> > + if (!ctx->quiet)
> > + {
> > + error_at (loc, "reference % failed");
> > + if (obj == NULL_TREE)
> > + inform (loc, "dynamic type %qT of its operand does not "
> > + "have an unambiguous public base class %qT",
> > + mdtype, type);
> > + else
> > + inform (loc, "static type %qT of its operand is a "
> > + "non-public base class of dynamic type %qT",
> > + objtype, type);
> > +
> > + }
> > + *non_constant_p = true;
> > + }
> > + return integer_zero_node;
> > + }
> > + else
> > + /* The result points to the TYPE object. */
> > + return cp_build_addr_expr (obj, complain);
> > +}
> > + /* Otherwise, if v points (refers) to a public base class subobject of
> > the
> > + most derived object, and the type of the most derived object has a
> > base
> > + class, of type C, that is unambiguous and public, the result points
> > + (refers) to the C subobject of the most derived object.
> > +
> > + But it can also be an invalid case. */
>
> And I think we need to fall through to this code if the hint turns out to be
> wrong, i.e. V is a public base of C, but v is not that subobject, but rather
> a sibling base of C, like
True. HINT is really just an optimization hint, nothing more. I've adjusted
the code to fall through to the normal processing if the HINT >= 0 or -3 case
doesn't succeed.
> struct A { virtual void f(); };
> struct B1: A { };
> struct B2: A { };
> struct C: B1, B2 { };
> int main()
> {
> C c;
> A* ap = (B1*)c;
> constexpr auto p = dynamic_cast(ap); // should succeed
> }
Whew, there's always One More Case. :/ New constexpr-dynamic16.c covers it.
> > --- /dev/null
> > +++ gcc/testsuite/g++.dg/cpp2a/constexpr-dynamic11.C
> > @@ -0,0 +1,34 @@
> > +// PR c++/88337 - Implement P1327R1: Allow dynamic_cast/typeid in
> > constexpr.
> > +// { dg-do compile { target c++2a } }
> > +
> > +// dynamic_cast in a constructor.
> > +
> > +struct V {
> > + virtual void f();
> > +};
> > +
> > +struct A : V { };
> > +
> > +struct B : V {
> > + constexpr B(V*, A*);
> > +};
> > +
> > +struct D : A, B {
> > + constexpr D() : B((A*)this, this) { } // { dg-message "in 'constexpr'
> > expansion of" }
> > +};
> > +
> > +constexpr B::B(V* v, A* a)
> > +{
> > + // well-defined: v of type V*, V base of B results in B*
> > + B* b = dynamic_cast(v);
> > + if (b != nullptr)
> > +__builtin_abort ();
> > +
> > + B& br = dynamic_cast(*v); // { dg-error "reference .dynamic_cast.
> > failed" }
> > +// { dg-message "dynamic type .A. of its operand does not have an
> > unambiguous public base class .B." "" { target *-*-* } .-1 }
> > +
> > + // FIXME: UB in constexpr should be detected.
> > + dynamic_cast(a); // undefined behavior, a has type A*, A
> > not a base of B
>
> What undefined behavior? Seems to me the dynamic_cast should just fail and
> return a null pointer.
This is :
"If the operand of the dynamic_cast refers to the object under construction
or destruction and the static type of the operand is not a pointer to or
object of the constructor or destructor's own class or one of its bases, the
dynamic_cast results in undefined behavior."
Re: AW: [PATCH] m68k architecture: support ccmode + lra
On Sat, 2019-12-14 at 15:33 +0100, John Paul Adrian Glaubitz wrote: > Hi Stefan! > > > The problems are in the gcc implementation > > > > - the lra implementation is buggy > > - the compare elimination can't handle parallel's containing a compare > > - df-core considers parallel's containing a compare also as a USE > > - some optimizations/mechanisms do only work if HAVE_CC0 is defined > > - way more ... > > Are you talking about GCC in general or about the m68k backend? > > In any case, I would appreciate it if you could file bug reports where you > are seeing issues and put me in the CC of these bug reports. Absolutely true for both cases. > > We are planning to have another Bountysource campaign for the m68k > backend to fund the LRA transition and - if necessary - fix other bugs > in the backend, so constructive feedback is necessary and absolutely > welcome. That may not be necessary. I wouldn't expect the conversion to LRA for m68k to be terribly hard. I would expect some slight problems with operand predicates/constraints as LRA is a bit more strict, but they're usually not terribly hard to track down. What's also considerably easier is you can flip LRA on and iterate on issues easily -- that wasn't really possible with the cc0 transition. > I can live with the fact that my patch was refuted since I simply use > > my *working* fork, where I fixed the issues mentioned above. > > I would prefer getting all bugs in GCC reported to the GCC Bugzilla so that > they can be resolved in GCC itself. We don't gain anything if important bug > fixes are found in forks only. Exactly. jeff
[committed] fix typos in attribute access
Committed in r279398.
Martin
gcc/ChangeLog:
* doc/extend.texi (attribute access): Correct typos.
Index: gcc/doc/extend.texi
===
--- gcc/doc/extend.texi (revision 279397)
+++ gcc/doc/extend.texi (working copy)
@@ -2489,10 +2489,10 @@ The following attributes are supported on most tar
@itemx access (@var{access-mode}, @var{ref-index}, @var{size-index})
The @code{access} attribute enables the detection of invalid or unsafe
-accesses by functions to which they apply to or their callers, as well
-as write-only accesses to objects that are never read from. Such accesses
+accesses by functions to which they apply or their callers, as well as
+write-only accesses to objects that are never read from. Such accesses
may be diagnosed by warnings such as @option{-Wstringop-overflow},
-@option{-Wunnitialized}, @option{-Wunused}, and others.
+@option{-Wuninitialized}, @option{-Wunused}, and others.
The @code{access} attribute specifies that a function to whose by-reference
arguments the attribute applies accesses the referenced object according to
@@ -2501,13 +2501,13 @@ one of three names: @code{read_only}, @code{read_w
The remaining two are positional arguments.
The required @var{ref-index} positional argument denotes a function
-argument of pointer (or in C++, refeference) type that is subject to
+argument of pointer (or in C++, reference) type that is subject to
the access. The same pointer argument can be referenced by at most one
distinct @code{access} attribute.
The optional @var{size-index} positional argument denotes a function
argument of integer type that specifies the maximum size of the access.
-The size is the number of elements of the type refefenced by @var{ref-index},
+The size is the number of elements of the type referenced by @var{ref-index},
or the number of bytes when the pointer type is @code{void*}. When no
@var{size-index} argument is specified, the pointer argument must be either
null or point to a space that is suitably aligned and large for at least one
@@ -2520,10 +2520,10 @@ applies is used to read the referenced object but
the argument specifying the size of the access denoted by @var{size-index}
is zero, the referenced object must be initialized. The mode implies
a stronger guarantee than the @code{const} qualifier which, when cast away
-from a pointer, does not prevent a function from modifying the pointed-to
-object. Examples of the use of the @code{read_only} access mode is
-the argument to the @code{puts} function, or the second and third arguments
-to the @code{memcpy} function.
+from a pointer, does not prevent the pointed-to object from being modified.
+Examples of the use of the @code{read_only} access mode is the argument to
+the @code{puts} function, or the second and third arguments to
+the @code{memcpy} function.
@smallexample
__attribute__ ((access (read_only))) int puts (const char*);
@@ -2534,7 +2534,7 @@ The @code{read_write} access mode applies to argum
without the @code{const} qualifier. It specifies that the pointer to which
it applies is used to both read and write the referenced object. Unless
the argument specifying the size of the access denoted by @var{size-index}
-is zero, the object refrenced by the pointer must be initialized. An example
+is zero, the object referenced by the pointer must be initialized. An example
of the use of the @code{read_write} access mode is the first argument to
the @code{strcat} function.
Re: [PATCH] Fix out of bounds array access in the preprocessor (PR preprocessor/92919)
On 12/14/19 3:09 AM, Jakub Jelinek wrote:
Hi!
wide_str_to_charconst function relies on the string passed to it
having at least two wide characters, the one we are looking for and
the terminating NUL. The empty wide character literal like
L'' or u'' or U'' is handled earlier and will not reach this function,
but unfortunately for
const char16_t p = u'\U00110003';
while we do emit an error wide_str_to_charconst is called with a string
that contains just the NUL terminator and nothing else. That is because
U110003 is too large and can't be represented even as a surrogate pair
in char16_t, but the handling of it doesn't give up on the whole string,
because other wide characters could be fine. Say u'a\U00110003' would
be passed to wide_str_to_charconst after diagnosing an error
because the too large char would be thrown away and we'd end up with
u'a'.
The following patch fixes it by just checking for this condition and
punting. I think it is undesirable to print further error.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
2019-12-13 Jakub Jelinek
PR preprocessor/92919
* charset.c (wide_str_to_charconst): If str contains just the
NUL terminator, punt quietly.
--- libcpp/charset.c.jj 2019-12-10 00:56:07.552291870 +0100
+++ libcpp/charset.c2019-12-13 12:23:59.096150225 +0100
@@ -1970,6 +1970,17 @@ wide_str_to_charconst (cpp_reader *pfile
size_t off, i;
cppchar_t result = 0, c;
+ if (str.len <= nbwc)
+{
+ /* Error recovery, if no errors have been diagnosed previously,
+there should be at least two wide characters. Empty literals
+are diagnosed earlier and we can get just the zero terminator
+only if there were errors diagnosed during conversion. */
+ *pchars_seen = 0;
+ *unsignedp = 0;
+ return 0;
+}
+
/* This is finicky because the string is in the target's byte order,
which may not be our byte order. Only the last character, ignoring
the NUL terminator, is relevant. */
Jakub
Re: AW: [PATCH] m68k architecture: support ccmode + lra
Hi Stefan! > The problems are in the gcc implementation > > - the lra implementation is buggy > - the compare elimination can't handle parallel's containing a compare > - df-core considers parallel's containing a compare also as a USE > - some optimizations/mechanisms do only work if HAVE_CC0 is defined > - way more ... Are you talking about GCC in general or about the m68k backend? In any case, I would appreciate it if you could file bug reports where you are seeing issues and put me in the CC of these bug reports. > And the current implementation is IMHO unusable for lra since it did not > introduce a CC register to track clobbering. So it's a dead end. I assume you are talking about Bernd's m68k cc0 transition? We are planning to have another Bountysource campaign for the m68k backend to fund the LRA transition and - if necessary - fix other bugs in the backend, so constructive feedback is necessary and absolutely welcome. On every bug that is documented in GCC Bugzilla, we can put a bounty if necessary. > I can live with the fact that my patch was refuted since I simply use > my *working* fork, where I fixed the issues mentioned above. I would prefer getting all bugs in GCC reported to the GCC Bugzilla so that they can be resolved in GCC itself. We don't gain anything if important bug fixes are found in forks only. Adrian -- .''`. John Paul Adrian Glaubitz : :' : Debian Developer - glaub...@debian.org `. `' Freie Universitaet Berlin - glaub...@physik.fu-berlin.de `-GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
Re: Add a compatible_vector_types_p target hook
On December 14, 2019 11:43:48 AM GMT+01:00, Richard Sandiford
wrote:
>Richard Biener writes:
>> On December 13, 2019 10:12:40 AM GMT+01:00, Richard Sandiford
> wrote:
>>>Richard Biener writes:
>>>The AArch64 port emits an error if calls pass values of SVE type
>to
>an
>>>unprototyped function. To do that we need to know whether the
>>>value
>>>really is an SVE type rathr than a plain vector.
>>>
>>>For varags the ABI is the same for 256 bits+. But we'll have the
>>>same problem there once we support -msve-vector-bits=128, since
>the
>>>layout of SVE and Advanced SIMD vectors differ for big-endian.
>>
>> But then why don't you have different modes?
>
>Yeah, true, modes will probably help for the Advanced SIMD/SVE
>difference. But from a vector value POV, a vector of 4 ints is a
>vector
>of 4 ints, so even distinguishing based on the mode is artificial.
True.
>SVE is AFAIK the first target to have different modes for
>potentially
>the "same" vector type, and I had to add new infrastructure to
>allow
>targets to define multiple modes of the same size. So the fact
>that
>gimple distinguishes otherwise identical vectors based on mode is a
>relatively recent thing. AFAIK it just fell out in the wash rather
>than being deliberately planned. It happens to be convenient in
>this
>context, but it hasn't been important until now.
>
>The hook doesn't seem any worse than distinguishing based on the
>>>mode.
>Another way to avoid this would have been to define separate SVE
>>>modes
>for the predefined vectors. The big downside of that is that we'd
>>>end
>up doubling the number of SVE patterns.
>
>Extra on-the-side metadata is going to be easy to drop
>accidentally,
>and this is something we need for correctness rather than
>>>optimisation.
Still selecting the ABI during call expansion only and based on
>>>values types at that point is fragile.
>>>
>>>Agreed. But it's fragile in general, not just for this case.
>Changing
>>>something as fundamental as that would be a lot of work and seems
>>>likely
>>>to introduce accidental ABI breakage.
>>>
The frontend are in charge of specifying the actual argument type
>and
at that point the target may fix the ABI. The ABI can be recorded
>in
the calls fntype, either via its TYPE_ARG_TYPES or in more awkward
ways for varargs functions (in full generality that would mean
attaching varargs ABI meta to each call).
The alternative is to have an actual argument type vector
>associated
with each call.
>>>
>>>I think multiple pieces of gimple code would then have to cope with
>>>that
>>>as a special case. E.g. if:
>>>
>>> void foo (int, ...);
>>>
>>> type1 a;
>>> b = VIEW_CONVERT_EXPR (a);
>>> if (a)
>>> foo (1, a);
>>> else
>>> foo (1, b);
>>>
>>>gets converted to:
>>>
>>> if (a)
>>> foo (1, a);
>>> else
>>> foo (1, a);
>>>
>>>on the basis that type1 and type2 are "the same" despite having
>>>different calling conventions, we have to be sure that the calls
>>>are not treated as equivalent:
>>>
>>> foo (1, a);
>>>
>>>Things like IPA clones would also need to handle this specially.
>>>Anything that generates new calls based on old ones will need
>>>to copy this information too.
>>>
>>>This also sounds like it would be fragile and seems a bit too
>>>invasive for stage 3.
>>
>> But we are already relying on this to work (fntype non-propagation)
>because function pointer conversions are dropped on the floor.
>>
>> The real change would be introducing (per call) fntype for calls to
>unprototyped functions and somehow dealing with varargs.
>
>It looks like this itself relies on useless_type_conversion_p,
>is that right? E.g. we have things like:
>
>bool
>func_checker::compare_gimple_call (gcall *s1, gcall *s2)
>{
> ...
> tree fntype1 = gimple_call_fntype (s1);
> tree fntype2 = gimple_call_fntype (s2);
> if ((fntype1 && !fntype2)
> || (!fntype1 && fntype2)
> || (fntype1 && !types_compatible_p (fntype1, fntype2)))
>return return_false_with_msg ("call function types are not
>compatible");
>
>and useless_type_conversion_p has:
>
> else if ((TREE_CODE (inner_type) == FUNCTION_TYPE
> || TREE_CODE (inner_type) == METHOD_TYPE)
> && TREE_CODE (inner_type) == TREE_CODE (outer_type))
>{
> tree outer_parm, inner_parm;
>
> /* If the return types are not compatible bail out. */
> if (!useless_type_conversion_p (TREE_TYPE (outer_type),
> TREE_TYPE (inner_type)))
> return false;
>
> /* Method types should belong to a compatible base class. */
> if (TREE_CODE (inner_type) == METHOD_TYPE
> && !useless_type_conversion_p (TYPE_METHOD_BASETYPE (outer_type),
>TYPE_METHOD_BASETYPE (inner_type)))
> return false;
>
> /* A
Re: [PATCH] Fix up handling of __builtin_apply in ipa-pure-const (PR tree-optimization/92930)
On December 14, 2019 9:21:21 AM GMT+01:00, Jakub Jelinek
wrote:
>Hi!
>
>__builtin_apply calls some other function, so whether it is const, pure
>or
>none of that, whether it can throw or not etc. depends on whether that
>called function is const or pure or none of that, whether it can throw
>etc.
>So, assuming __builtin_apply is const is wrong. The following patch
>looks larger due to reformatting, but basically took the case
>BUILT_IN_APPLY:
>line out.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Ok.
Richard.
>If we really cared that much about a builtin almost nobody uses, we
>could
>analyze the called function, but this stuff is called from two
>different
>spots and I think from the other one the arguments of the
>__builtin_apply
>call aren't available.
>
>2019-12-14 Jakub Jelinek
>
> PR tree-optimization/92930
> * ipa-pure-const.c (special_builtin_state): Don't handle
> BUILT_IN_APPLY. Formatting fixes.
> (check_call): Formatting fixes.
>
> * gcc.dg/tree-ssa/pr92930.c: New test.
>
>--- gcc/ipa-pure-const.c.jj2019-10-30 10:49:35.075045961 +0100
>+++ gcc/ipa-pure-const.c 2019-12-13 18:52:53.822632062 +0100
>@@ -511,35 +511,34 @@ worse_state (enum pure_const_state_e *st
>but function using them is. */
> static bool
> special_builtin_state (enum pure_const_state_e *state, bool *looping,
>- tree callee)
>+ tree callee)
> {
> if (DECL_BUILT_IN_CLASS (callee) == BUILT_IN_NORMAL)
> switch (DECL_FUNCTION_CODE (callee))
> {
>- case BUILT_IN_RETURN:
>- case BUILT_IN_UNREACHABLE:
>- CASE_BUILT_IN_ALLOCA:
>- case BUILT_IN_STACK_SAVE:
>- case BUILT_IN_STACK_RESTORE:
>- case BUILT_IN_EH_POINTER:
>- case BUILT_IN_EH_FILTER:
>- case BUILT_IN_UNWIND_RESUME:
>- case BUILT_IN_CXA_END_CLEANUP:
>- case BUILT_IN_EH_COPY_VALUES:
>- case BUILT_IN_FRAME_ADDRESS:
>- case BUILT_IN_APPLY:
>- case BUILT_IN_APPLY_ARGS:
>- case BUILT_IN_ASAN_BEFORE_DYNAMIC_INIT:
>- case BUILT_IN_ASAN_AFTER_DYNAMIC_INIT:
>-*looping = false;
>-*state = IPA_CONST;
>-return true;
>- case BUILT_IN_PREFETCH:
>-*looping = true;
>-*state = IPA_CONST;
>-return true;
>- default:
>-break;
>+ case BUILT_IN_RETURN:
>+ case BUILT_IN_UNREACHABLE:
>+ CASE_BUILT_IN_ALLOCA:
>+ case BUILT_IN_STACK_SAVE:
>+ case BUILT_IN_STACK_RESTORE:
>+ case BUILT_IN_EH_POINTER:
>+ case BUILT_IN_EH_FILTER:
>+ case BUILT_IN_UNWIND_RESUME:
>+ case BUILT_IN_CXA_END_CLEANUP:
>+ case BUILT_IN_EH_COPY_VALUES:
>+ case BUILT_IN_FRAME_ADDRESS:
>+ case BUILT_IN_APPLY_ARGS:
>+ case BUILT_IN_ASAN_BEFORE_DYNAMIC_INIT:
>+ case BUILT_IN_ASAN_AFTER_DYNAMIC_INIT:
>+ *looping = false;
>+ *state = IPA_CONST;
>+ return true;
>+ case BUILT_IN_PREFETCH:
>+ *looping = true;
>+ *state = IPA_CONST;
>+ return true;
>+ default:
>+ break;
> }
> return false;
> }
>@@ -624,9 +623,10 @@ check_call (funct_state local, gcall *ca
> case BUILT_IN_LONGJMP:
> case BUILT_IN_NONLOCAL_GOTO:
> if (dump_file)
>-fprintf (dump_file, "longjmp and nonlocal goto is not
>const/pure\n");
>+fprintf (dump_file,
>+ "longjmp and nonlocal goto is not const/pure\n");
> local->pure_const_state = IPA_NEITHER;
>-local->looping = true;
>+ local->looping = true;
> break;
> default:
> break;
>@@ -1532,7 +1532,7 @@ propagate_pure_const (void)
> }
> }
> else if (special_builtin_state (_state, _looping,
>- y->decl))
>+y->decl))
> ;
> else
> state_from_flags (_state, _looping,
>--- gcc/testsuite/gcc.dg/tree-ssa/pr92930.c.jj 2019-12-13
>18:59:19.651732842 +0100
>+++ gcc/testsuite/gcc.dg/tree-ssa/pr92930.c2019-12-13
>19:00:34.829582072 +0100
>@@ -0,0 +1,19 @@
>+/* PR tree-optimization/92930 */
>+/* { dg-do compile { target untyped_assembly } } */
>+/* { dg-options "-O2 -fdump-tree-optimized" } */
>+/* { dg-final { scan-tree-dump "__builtin_apply " "optimized" } } */
>+/* { dg-final { scan-tree-dump "__builtin_apply_args" "optimized" } }
>*/
>+
>+void foo (int a, int b, int c, int d, int e, int f, int g);
>+
>+static void bar (int a, ...)
>+{
>+ __builtin_apply (foo, __builtin_apply_args (), 20);
>+}
>+
>+int
>+main ()
>+{
>+ bar (1024, 1025, 1026, 1027, 1028, 1029, 1030);
>+ return 0;
>+}
>
> Jakub
Re: [PATCH] Fix ipa_fn_summary_write for offloading LTO streaming (PR ipa/92357)
On December 14, 2019 10:11:33 AM GMT+01:00, Jakub Jelinek
wrote:
>Hi!
>
>As discussed on IRC and in the PR, we get an ICE during
>inline_read_section
>in the offloading lto1, because the size of streamed out and expected
>to be
>streamed in data doesn't match. This happens on the testcase from the
>PR
>where
> for (edge = cnode->callees; edge; edge = edge->next_callee)
>write_ipa_call_summary (ob, edge);
>writes summaries for 2 edges and
> for (e = node->callees; e; e = e->next_callee)
>read_ipa_call_summary (, e, info != NULL);
>has NULL node->callees and doesn't thus stream in anything.
>That is because
> for (int i = 0; i < lto_symtab_encoder_size (encoder); i++)
>{
>node = dyn_cast (lto_symtab_encoder_deref (encoder,
>i));
> if (node
> && ((node->thunk.thunk_p && !node->inlined_to)
> || lto_symtab_encoder_in_partition_p (encoder, node)))
>{
> output_outgoing_cgraph_edges (node->callees, ob, encoder);
> output_outgoing_cgraph_edges (node->indirect_calls, ob, encoder);
>}
>}
>didn't stream the callees out, as lto_symtab_encoder_in_partition_p
>(encoder, node)
>is false. All the other IPA writers that write function summaries only
>handle functions that pass this predicate, using the
>lsei_*_function_in_partition
>iterators, but ipa_fn_summary_write streams all.
>
>Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
>regtested on x86_64-linux with nvptx-none offloading, ok for trunk?
Ok.
Richard.
>Note, there is another issue unrelated to this that still needs fixing.
>
>2019-12-14 Jakub Jelinek
>
> PR ipa/92357
> * ipa-fnsummary.c (ipa_fn_summary_write): Use
> lto_symtab_encoder_iterator with lsei_start_function_in_partition and
> lsei_next_function_in_partition instead of walking all cgraph nodes
> in encoder.
>
>--- gcc/ipa-fnsummary.c.jj 2019-12-05 14:02:20.559570378 +0100
>+++ gcc/ipa-fnsummary.c2019-12-13 18:01:08.344828332 +0100
>@@ -4364,24 +4364,24 @@ static void
> ipa_fn_summary_write (void)
> {
>struct output_block *ob = create_output_block
>(LTO_section_ipa_fn_summary);
>+ lto_symtab_encoder_iterator lsei;
> lto_symtab_encoder_t encoder = ob->decl_state->symtab_node_encoder;
> unsigned int count = 0;
>- int i;
>
>- for (i = 0; i < lto_symtab_encoder_size (encoder); i++)
>+ for (lsei = lsei_start_function_in_partition (encoder); !lsei_end_p
>(lsei);
>+ lsei_next_function_in_partition ())
> {
>- symtab_node *snode = lto_symtab_encoder_deref (encoder, i);
>- cgraph_node *cnode = dyn_cast (snode);
>- if (cnode && cnode->definition && !cnode->alias)
>+ cgraph_node *cnode = lsei_cgraph_node (lsei);
>+ if (cnode->definition && !cnode->alias)
> count++;
> }
> streamer_write_uhwi (ob, count);
>
>- for (i = 0; i < lto_symtab_encoder_size (encoder); i++)
>+ for (lsei = lsei_start_function_in_partition (encoder); !lsei_end_p
>(lsei);
>+ lsei_next_function_in_partition ())
> {
>- symtab_node *snode = lto_symtab_encoder_deref (encoder, i);
>- cgraph_node *cnode = dyn_cast (snode);
>- if (cnode && cnode->definition && !cnode->alias)
>+ cgraph_node *cnode = lsei_cgraph_node (lsei);
>+ if (cnode->definition && !cnode->alias)
> {
> class ipa_fn_summary *info = ipa_fn_summaries->get (cnode);
> class ipa_size_summary *size_info = ipa_size_summaries->get (cnode);
>
> Jakub
Re: Add a compatible_vector_types_p target hook
Richard Biener writes:
> On December 13, 2019 10:12:40 AM GMT+01:00, Richard Sandiford
> wrote:
>>Richard Biener writes:
>>The AArch64 port emits an error if calls pass values of SVE type to
an
>>unprototyped function. To do that we need to know whether the
>>value
>>really is an SVE type rathr than a plain vector.
>>
>>For varags the ABI is the same for 256 bits+. But we'll have the
>>same problem there once we support -msve-vector-bits=128, since the
>>layout of SVE and Advanced SIMD vectors differ for big-endian.
>
> But then why don't you have different modes?
Yeah, true, modes will probably help for the Advanced SIMD/SVE
difference. But from a vector value POV, a vector of 4 ints is a
vector
of 4 ints, so even distinguishing based on the mode is artificial.
>>>
>>> True.
>>>
SVE is AFAIK the first target to have different modes for potentially
the "same" vector type, and I had to add new infrastructure to allow
targets to define multiple modes of the same size. So the fact that
gimple distinguishes otherwise identical vectors based on mode is a
relatively recent thing. AFAIK it just fell out in the wash rather
than being deliberately planned. It happens to be convenient in this
context, but it hasn't been important until now.
The hook doesn't seem any worse than distinguishing based on the
>>mode.
Another way to avoid this would have been to define separate SVE
>>modes
for the predefined vectors. The big downside of that is that we'd
>>end
up doubling the number of SVE patterns.
Extra on-the-side metadata is going to be easy to drop accidentally,
and this is something we need for correctness rather than
>>optimisation.
>>>
>>> Still selecting the ABI during call expansion only and based on
>>values types at that point is fragile.
>>
>>Agreed. But it's fragile in general, not just for this case. Changing
>>something as fundamental as that would be a lot of work and seems
>>likely
>>to introduce accidental ABI breakage.
>>
>>> The frontend are in charge of specifying the actual argument type and
>>> at that point the target may fix the ABI. The ABI can be recorded in
>>> the calls fntype, either via its TYPE_ARG_TYPES or in more awkward
>>> ways for varargs functions (in full generality that would mean
>>> attaching varargs ABI meta to each call).
>>>
>>> The alternative is to have an actual argument type vector associated
>>> with each call.
>>
>>I think multiple pieces of gimple code would then have to cope with
>>that
>>as a special case. E.g. if:
>>
>> void foo (int, ...);
>>
>> type1 a;
>> b = VIEW_CONVERT_EXPR (a);
>> if (a)
>> foo (1, a);
>> else
>> foo (1, b);
>>
>>gets converted to:
>>
>> if (a)
>> foo (1, a);
>> else
>> foo (1, a);
>>
>>on the basis that type1 and type2 are "the same" despite having
>>different calling conventions, we have to be sure that the calls
>>are not treated as equivalent:
>>
>> foo (1, a);
>>
>>Things like IPA clones would also need to handle this specially.
>>Anything that generates new calls based on old ones will need
>>to copy this information too.
>>
>>This also sounds like it would be fragile and seems a bit too
>>invasive for stage 3.
>
> But we are already relying on this to work (fntype non-propagation) because
> function pointer conversions are dropped on the floor.
>
> The real change would be introducing (per call) fntype for calls to
> unprototyped functions and somehow dealing with varargs.
It looks like this itself relies on useless_type_conversion_p,
is that right? E.g. we have things like:
bool
func_checker::compare_gimple_call (gcall *s1, gcall *s2)
{
...
tree fntype1 = gimple_call_fntype (s1);
tree fntype2 = gimple_call_fntype (s2);
if ((fntype1 && !fntype2)
|| (!fntype1 && fntype2)
|| (fntype1 && !types_compatible_p (fntype1, fntype2)))
return return_false_with_msg ("call function types are not compatible");
and useless_type_conversion_p has:
else if ((TREE_CODE (inner_type) == FUNCTION_TYPE
|| TREE_CODE (inner_type) == METHOD_TYPE)
&& TREE_CODE (inner_type) == TREE_CODE (outer_type))
{
tree outer_parm, inner_parm;
/* If the return types are not compatible bail out. */
if (!useless_type_conversion_p (TREE_TYPE (outer_type),
TREE_TYPE (inner_type)))
return false;
/* Method types should belong to a compatible base class. */
if (TREE_CODE (inner_type) == METHOD_TYPE
&& !useless_type_conversion_p (TYPE_METHOD_BASETYPE (outer_type),
TYPE_METHOD_BASETYPE (inner_type)))
return false;
/* A conversion to an unprototyped argument list is ok. */
if (!prototype_p (outer_type))
return true;
/* If the unqualified argument types are compatible the conversion
is
[PATCH] Fix ipa_fn_summary_write for offloading LTO streaming (PR ipa/92357)
Hi!
As discussed on IRC and in the PR, we get an ICE during inline_read_section
in the offloading lto1, because the size of streamed out and expected to be
streamed in data doesn't match. This happens on the testcase from the PR
where
for (edge = cnode->callees; edge; edge = edge->next_callee)
write_ipa_call_summary (ob, edge);
writes summaries for 2 edges and
for (e = node->callees; e; e = e->next_callee)
read_ipa_call_summary (, e, info != NULL);
has NULL node->callees and doesn't thus stream in anything.
That is because
for (int i = 0; i < lto_symtab_encoder_size (encoder); i++)
{
node = dyn_cast (lto_symtab_encoder_deref (encoder, i));
if (node
&& ((node->thunk.thunk_p && !node->inlined_to)
|| lto_symtab_encoder_in_partition_p (encoder, node)))
{
output_outgoing_cgraph_edges (node->callees, ob, encoder);
output_outgoing_cgraph_edges (node->indirect_calls, ob, encoder);
}
}
didn't stream the callees out, as lto_symtab_encoder_in_partition_p (encoder,
node)
is false. All the other IPA writers that write function summaries only
handle functions that pass this predicate, using the
lsei_*_function_in_partition
iterators, but ipa_fn_summary_write streams all.
Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
regtested on x86_64-linux with nvptx-none offloading, ok for trunk?
Note, there is another issue unrelated to this that still needs fixing.
2019-12-14 Jakub Jelinek
PR ipa/92357
* ipa-fnsummary.c (ipa_fn_summary_write): Use
lto_symtab_encoder_iterator with lsei_start_function_in_partition and
lsei_next_function_in_partition instead of walking all cgraph nodes
in encoder.
--- gcc/ipa-fnsummary.c.jj 2019-12-05 14:02:20.559570378 +0100
+++ gcc/ipa-fnsummary.c 2019-12-13 18:01:08.344828332 +0100
@@ -4364,24 +4364,24 @@ static void
ipa_fn_summary_write (void)
{
struct output_block *ob = create_output_block (LTO_section_ipa_fn_summary);
+ lto_symtab_encoder_iterator lsei;
lto_symtab_encoder_t encoder = ob->decl_state->symtab_node_encoder;
unsigned int count = 0;
- int i;
- for (i = 0; i < lto_symtab_encoder_size (encoder); i++)
+ for (lsei = lsei_start_function_in_partition (encoder); !lsei_end_p (lsei);
+ lsei_next_function_in_partition ())
{
- symtab_node *snode = lto_symtab_encoder_deref (encoder, i);
- cgraph_node *cnode = dyn_cast (snode);
- if (cnode && cnode->definition && !cnode->alias)
+ cgraph_node *cnode = lsei_cgraph_node (lsei);
+ if (cnode->definition && !cnode->alias)
count++;
}
streamer_write_uhwi (ob, count);
- for (i = 0; i < lto_symtab_encoder_size (encoder); i++)
+ for (lsei = lsei_start_function_in_partition (encoder); !lsei_end_p (lsei);
+ lsei_next_function_in_partition ())
{
- symtab_node *snode = lto_symtab_encoder_deref (encoder, i);
- cgraph_node *cnode = dyn_cast (snode);
- if (cnode && cnode->definition && !cnode->alias)
+ cgraph_node *cnode = lsei_cgraph_node (lsei);
+ if (cnode->definition && !cnode->alias)
{
class ipa_fn_summary *info = ipa_fn_summaries->get (cnode);
class ipa_size_summary *size_info = ipa_size_summaries->get (cnode);
Jakub
[Darwin, PPC, committed] Use Darwin9 bundle header for Rosetta builds.
On Darwin10 it's possible to make a 32b PPC build using the
'Rosetta' emulator. However, these builds need to make use of
Darwin9 crts (for exes, dylibs and bundles). This adds the
change to cater for bundles.
tested on powerpc-darwin10 cross from x86-64-darwin16 and on
x86-64-darwin16.
gcc/ChangeLog:
2019-12-14 Iain Sandoe
* config/darwin.h (DARWIN_EXTRA_SPECS): Add new
bundle spec. (DARWIN_BUNDLE1_SPEC): New.
(STARTFILE_SPEC): Use darwin bundle spec.
* config/rs6000/darwin.h (DARWIN_BUNDLE1_SPEC): New.
(DARWIN_DYLIB1_SPEC): Delete duplicate.
diff --git a/gcc/config/darwin.h b/gcc/config/darwin.h
index 8eb8edf..546336e 100644
--- a/gcc/config/darwin.h
+++ b/gcc/config/darwin.h
@@ -401,9 +401,7 @@ extern GTY(()) int darwin_ms_struct;
#undef STARTFILE_SPEC
#define STARTFILE_SPEC \
"%{Zdynamiclib: %(darwin_dylib1) %{fgnu-tm: -lcrttms.o}} \
- %{!Zdynamiclib:%{Zbundle:%{!static: \
- %:version-compare(< 10.6 mmacosx-version-min= -lbundle1.o) \
- %{fgnu-tm: -lcrttms.o}}}\
+ %{!Zdynamiclib:%{Zbundle:%(darwin_bundle1)} \
%{!Zbundle:%{pg:%{static:-lgcrt0.o} \
%{!static:%{object:-lgcrt0.o} \
%{!object:%{preload:-lgcrt0.o} \
@@ -425,7 +423,8 @@ extern GTY(()) int darwin_ms_struct;
{ "darwin_crt1", DARWIN_CRT1_SPEC }, \
{ "darwin_crt2", DARWIN_CRT2_SPEC }, \
{ "darwin_crt3", DARWIN_CRT3_SPEC }, \
- { "darwin_dylib1", DARWIN_DYLIB1_SPEC },
+ { "darwin_dylib1", DARWIN_DYLIB1_SPEC }, \
+ { "darwin_bundle1", DARWIN_BUNDLE1_SPEC },
#define DARWIN_CRT1_SPEC \
"%:version-compare(!> 10.5 mmacosx-version-min= -lcrt1.o)\
@@ -447,6 +446,10 @@ extern GTY(()) int darwin_ms_struct;
"%:version-compare(!> 10.5 mmacosx-version-min= -ldylib1.o) \
%:version-compare(>< 10.5 10.6 mmacosx-version-min= -ldylib1.10.5.o)"
+#define DARWIN_BUNDLE1_SPEC \
+"%{!static:%:version-compare(< 10.6 mmacosx-version-min= -lbundle1.o) \
+ %{fgnu-tm: -lcrttms.o}}"
+
#ifdef HAVE_AS_MMACOSX_VERSION_MIN_OPTION
/* Emit macosx version (but only major). */
#define ASM_MMACOSX_VERSION_MIN_SPEC \
diff --git a/gcc/config/rs6000/darwin.h b/gcc/config/rs6000/darwin.h
index 8cf25b0..5f5a6ca 100644
--- a/gcc/config/rs6000/darwin.h
+++ b/gcc/config/rs6000/darwin.h
@@ -173,12 +173,18 @@
%:version-compare(!> 10.4 mmacosx-version-min= crt3_2.o%s) \
}}"
-/* We need to jam the dylib1 to 10.5 for 10.6 (Rosetta) use. */
+/* As for crt1, we need to force the dylib crt for 10.6. */
#undef DARWIN_DYLIB1_SPEC
#define DARWIN_DYLIB1_SPEC \
"%:version-compare(!> 10.5 mmacosx-version-min= -ldylib1.o) \
%:version-compare(>< 10.5 10.7 mmacosx-version-min= -ldylib1.10.5.o)"
+/* Likewise, the bundle crt. */
+#undef DARWIN_BUNDLE1_SPEC
+#define DARWIN_BUNDLE1_SPEC \
+"%{!static:%:version-compare(< 10.7 mmacosx-version-min= -lbundle1.o) \
+ %{fgnu-tm: -lcrttms.o}}"
+
/* The PPC regs save/restore functions are leaves and could, conceivably
be used by the tm destructor. */
#undef ENDFILE_SPEC
@@ -191,12 +197,6 @@
{ "darwin_crt2", DARWIN_CRT2_SPEC }, \
{ "darwin_subarch", DARWIN_SUBARCH_SPEC },
-/* We need to jam the dylib crt to 10.5 for 10.6 (Rosetta) use. */
-#undef DARWIN_DYLIB1_SPEC
-#define DARWIN_DYLIB1_SPEC \
- "%:version-compare(!> 10.5 mmacosx-version-min= -ldylib1.o) \
- %:version-compare(>< 10.5 10.7 mmacosx-version-min= -ldylib1.10.5.o)"
-
/* Output a .machine directive. */
#undef TARGET_ASM_FILE_START
#define TARGET_ASM_FILE_START rs6000_darwin_file_start
[PATCH] Fix up handling of __builtin_apply in ipa-pure-const (PR tree-optimization/92930)
Hi!
__builtin_apply calls some other function, so whether it is const, pure or
none of that, whether it can throw or not etc. depends on whether that
called function is const or pure or none of that, whether it can throw etc.
So, assuming __builtin_apply is const is wrong. The following patch
looks larger due to reformatting, but basically took the case BUILT_IN_APPLY:
line out.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
If we really cared that much about a builtin almost nobody uses, we could
analyze the called function, but this stuff is called from two different
spots and I think from the other one the arguments of the __builtin_apply
call aren't available.
2019-12-14 Jakub Jelinek
PR tree-optimization/92930
* ipa-pure-const.c (special_builtin_state): Don't handle
BUILT_IN_APPLY. Formatting fixes.
(check_call): Formatting fixes.
* gcc.dg/tree-ssa/pr92930.c: New test.
--- gcc/ipa-pure-const.c.jj 2019-10-30 10:49:35.075045961 +0100
+++ gcc/ipa-pure-const.c2019-12-13 18:52:53.822632062 +0100
@@ -511,35 +511,34 @@ worse_state (enum pure_const_state_e *st
but function using them is. */
static bool
special_builtin_state (enum pure_const_state_e *state, bool *looping,
- tree callee)
+ tree callee)
{
if (DECL_BUILT_IN_CLASS (callee) == BUILT_IN_NORMAL)
switch (DECL_FUNCTION_CODE (callee))
{
- case BUILT_IN_RETURN:
- case BUILT_IN_UNREACHABLE:
- CASE_BUILT_IN_ALLOCA:
- case BUILT_IN_STACK_SAVE:
- case BUILT_IN_STACK_RESTORE:
- case BUILT_IN_EH_POINTER:
- case BUILT_IN_EH_FILTER:
- case BUILT_IN_UNWIND_RESUME:
- case BUILT_IN_CXA_END_CLEANUP:
- case BUILT_IN_EH_COPY_VALUES:
- case BUILT_IN_FRAME_ADDRESS:
- case BUILT_IN_APPLY:
- case BUILT_IN_APPLY_ARGS:
- case BUILT_IN_ASAN_BEFORE_DYNAMIC_INIT:
- case BUILT_IN_ASAN_AFTER_DYNAMIC_INIT:
- *looping = false;
- *state = IPA_CONST;
- return true;
- case BUILT_IN_PREFETCH:
- *looping = true;
- *state = IPA_CONST;
- return true;
- default:
- break;
+ case BUILT_IN_RETURN:
+ case BUILT_IN_UNREACHABLE:
+ CASE_BUILT_IN_ALLOCA:
+ case BUILT_IN_STACK_SAVE:
+ case BUILT_IN_STACK_RESTORE:
+ case BUILT_IN_EH_POINTER:
+ case BUILT_IN_EH_FILTER:
+ case BUILT_IN_UNWIND_RESUME:
+ case BUILT_IN_CXA_END_CLEANUP:
+ case BUILT_IN_EH_COPY_VALUES:
+ case BUILT_IN_FRAME_ADDRESS:
+ case BUILT_IN_APPLY_ARGS:
+ case BUILT_IN_ASAN_BEFORE_DYNAMIC_INIT:
+ case BUILT_IN_ASAN_AFTER_DYNAMIC_INIT:
+ *looping = false;
+ *state = IPA_CONST;
+ return true;
+ case BUILT_IN_PREFETCH:
+ *looping = true;
+ *state = IPA_CONST;
+ return true;
+ default:
+ break;
}
return false;
}
@@ -624,9 +623,10 @@ check_call (funct_state local, gcall *ca
case BUILT_IN_LONGJMP:
case BUILT_IN_NONLOCAL_GOTO:
if (dump_file)
- fprintf (dump_file, "longjmp and nonlocal goto is not
const/pure\n");
+ fprintf (dump_file,
+ "longjmp and nonlocal goto is not const/pure\n");
local->pure_const_state = IPA_NEITHER;
-local->looping = true;
+ local->looping = true;
break;
default:
break;
@@ -1532,7 +1532,7 @@ propagate_pure_const (void)
}
}
else if (special_builtin_state (_state, _looping,
- y->decl))
+ y->decl))
;
else
state_from_flags (_state, _looping,
--- gcc/testsuite/gcc.dg/tree-ssa/pr92930.c.jj 2019-12-13 18:59:19.651732842
+0100
+++ gcc/testsuite/gcc.dg/tree-ssa/pr92930.c 2019-12-13 19:00:34.829582072
+0100
@@ -0,0 +1,19 @@
+/* PR tree-optimization/92930 */
+/* { dg-do compile { target untyped_assembly } } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+/* { dg-final { scan-tree-dump "__builtin_apply " "optimized" } } */
+/* { dg-final { scan-tree-dump "__builtin_apply_args" "optimized" } } */
+
+void foo (int a, int b, int c, int d, int e, int f, int g);
+
+static void bar (int a, ...)
+{
+ __builtin_apply (foo, __builtin_apply_args (), 20);
+}
+
+int
+main ()
+{
+ bar (1024, 1025, 1026, 1027, 1028, 1029, 1030);
+ return 0;
+}
Jakub
[PATCH] Fix out of bounds array access in the preprocessor (PR preprocessor/92919)
Hi!
wide_str_to_charconst function relies on the string passed to it
having at least two wide characters, the one we are looking for and
the terminating NUL. The empty wide character literal like
L'' or u'' or U'' is handled earlier and will not reach this function,
but unfortunately for
const char16_t p = u'\U00110003';
while we do emit an error wide_str_to_charconst is called with a string
that contains just the NUL terminator and nothing else. That is because
U110003 is too large and can't be represented even as a surrogate pair
in char16_t, but the handling of it doesn't give up on the whole string,
because other wide characters could be fine. Say u'a\U00110003' would
be passed to wide_str_to_charconst after diagnosing an error
because the too large char would be thrown away and we'd end up with
u'a'.
The following patch fixes it by just checking for this condition and
punting. I think it is undesirable to print further error.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2019-12-13 Jakub Jelinek
PR preprocessor/92919
* charset.c (wide_str_to_charconst): If str contains just the
NUL terminator, punt quietly.
--- libcpp/charset.c.jj 2019-12-10 00:56:07.552291870 +0100
+++ libcpp/charset.c2019-12-13 12:23:59.096150225 +0100
@@ -1970,6 +1970,17 @@ wide_str_to_charconst (cpp_reader *pfile
size_t off, i;
cppchar_t result = 0, c;
+ if (str.len <= nbwc)
+{
+ /* Error recovery, if no errors have been diagnosed previously,
+there should be at least two wide characters. Empty literals
+are diagnosed earlier and we can get just the zero terminator
+only if there were errors diagnosed during conversion. */
+ *pchars_seen = 0;
+ *unsignedp = 0;
+ return 0;
+}
+
/* This is finicky because the string is in the target's byte order,
which may not be our byte order. Only the last character, ignoring
the NUL terminator, is relevant. */
Jakub
