[PATCH] ppc: testsuite: vec-mul requires vsx runtime
vec-mul is an execution test, but it only requires a powerpc_vsx_ok
effective target, which is enough only for compile tests. In order to
To check for runtime and execution environment support, we need to
require vsx_hw. Make that a condition for execution, but still
perform a compile test if the condition is not satisfied.
Regstrapped on ppc64le-linux-gnu, also tested on
x86_64-linux-gnu-x-ppc-vx7r2 (no vsx execution support)
Ok to install? gcc-12? gcc-11?
for gcc/testsuite/ChangeLog
* testsuite/powerpc/vec-mul.c: Run on target vsx_hw, just
compile otherwise.
---
gcc/testsuite/gcc.target/powerpc/vec-mul.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/gcc/testsuite/gcc.target/powerpc/vec-mul.c
b/gcc/testsuite/gcc.target/powerpc/vec-mul.c
index bfcaf80719d1d..11da86159723f 100644
--- a/gcc/testsuite/gcc.target/powerpc/vec-mul.c
+++ b/gcc/testsuite/gcc.target/powerpc/vec-mul.c
@@ -1,4 +1,5 @@
-/* { dg-do run } */
+/* { dg-do compile { target { ! vsx_hw } } } */
+/* { dg-do run { target vsx_hw } } */
/* { dg-require-effective-target powerpc_vsx_ok } */
/* { dg-options "-mvsx -O3" } */
--
Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
Free Software Activist GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts. Ask me about
[PATCH] [PR105324] libstdc++: testsuite: pr105324 requires FP from_char
The floating-point overloads of from_char are only declared if
_GLIBCXX_HAVE_USELOCALE is #defined as nonzero. That's exposed from
charconv as _cpp_lib_to_chars >= 201611L, so guard the test body with
that.
Regstrapped on ppc64le-linux-gnu, and tested on
x86_64-linux-gnu-x-ppc{,64}-vx7r2 (without _GLIBCXX_HAVE_USELOCALE).
Ok to install? gcc-12? gcc-11?
for libstdc++-v3/ChangeLog
PR c++/105324
* testsuite/20_util/from_chars/pr105324.cc: Guard test body
with conditional for floating-point overloads of from_char.
---
.../testsuite/20_util/from_chars/pr105324.cc |2 ++
1 file changed, 2 insertions(+)
diff --git a/libstdc++-v3/testsuite/20_util/from_chars/pr105324.cc
b/libstdc++-v3/testsuite/20_util/from_chars/pr105324.cc
index cecb17e41cc68..a9e08303ef49d 100644
--- a/libstdc++-v3/testsuite/20_util/from_chars/pr105324.cc
+++ b/libstdc++-v3/testsuite/20_util/from_chars/pr105324.cc
@@ -5,10 +5,12 @@
int main()
{
+#if _cpp_lib_to_chars >= 201611L // FP from_char not available otherwise.
// PR libstdc++/105324
// std::from_chars() assertion at floating_from_chars.cc:78
std::string s(512, '1');
s[1] = '.';
long double d;
std::from_chars(s.data(), s.data() + s.size(), d);
+#endif
}
--
Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
Free Software Activist GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts. Ask me about
Re: [PATCH] testsuite: vect: update unaligned message
On Apr 28, 2022, Richard Biener wrote:
> On Thu, 28 Apr 2022, Alexandre Oliva wrote:
>>
>> gcc.dg/vect/costmodel/ppc/costmodel-vect-31a.c covers ppc variants
>> that accept and reject misaligned accesses. The message that it
>> expects for rejection was removed in the gcc-11 development cycle by
>> commit r11-1969. The patch adjusted multiple tests to use the message
>> introduced in r11-1945, but missed this one.
>>
>> Regstrapped on powerpc64el-linux-gnu. Ok to install?
>>
>> Also tested on x86_64-linux-gnu-x-ppc64-vx7r2 with gcc-11. Ok for
>> gcc-11?
> OK.
I checked it in the trunk so far, but I missed the gcc-12 branching.
This is surely not critical for gcc-12, but it's so trivial that I
wonder if I could install it in both gcc-12 and gcc-11 branches and
forget about it, or wait until 12.1 is final. WDYT?
>> for gcc/testsuite/ChangeLog
>>
>> * gcc.dg/vect/costmodel/ppc/costmodel-vect-31a.c: Update
>> the expected message for the case in which unaligned accesses
>> are not allowed.
>> -/* { dg-final { scan-tree-dump-times "not vectorized: unsupported unaligned
>> store" 1 "vect" { target { ! vect_hw_misalign } } } } */
>> +/* { dg-final { scan-tree-dump-times "unsupported unaligned access" 1
>> "vect" { target { ! vect_hw_misalign } } } } */
--
Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
Free Software Activist GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts. Ask me about
[PATCH v2] libstdc++: ppc: conditionalize vsx-only simd intrinsics
On May 2, 2022, Segher Boessenkool wrote:
> Send full patches always please.
I'll try to remember that. In case I fail, I hope you won't mind too
much reminding me.
(You'd also asked me not to send patches as followups, but... revised
versions of a patch still belong in the same thread, right?)
Here's the updated full patch. I tried to find earlier references to
this partial specialization of template struct __intrinsic_type in
ChangeLogs, to no avail. (That was why I went for the more abstract
ChangeLog entry before.)
libstdc++: ppc: conditionalize vsx-only simd intrinsics
From: Alexandre Oliva
libstdc++'s bits/simd.h section for PPC (Altivec) defines various
intrinsic vector types that are only available along with VSX: 64-bit
long double, double, (un)signed long long, and 64-bit (un)signed long.
experimental/simd/standard_abi_usable{,_2}.cc tests error out
reporting the unmet requirements when the target cpu doesn't enable
VSX. Make the reported instrinsic types conditional on VSX so that
can be used on ppc variants that do not have VSX
support.
Regstrapping on powerpc64le-linux-gnu. Ok for trunk? gcc-12? gcc-11?
for libstdc++-v3/ChangeLog
* include/experimental/bits/simd.h [__ALTIVEC__]: Require VSX
for double, long long, and 64-bit long intrinsic types.
[__ALTIVEC__] (__intrinsic_type): Mention 128-bit in
preexisting long double diagnostic, adjust no-VSX double
diagnostic to cover 64-bit long double as well.
---
libstdc++-v3/include/experimental/bits/simd.h | 13 ++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/libstdc++-v3/include/experimental/bits/simd.h
b/libstdc++-v3/include/experimental/bits/simd.h
index 82e9841195e1d..349726a16d71c 100644
--- a/libstdc++-v3/include/experimental/bits/simd.h
+++ b/libstdc++-v3/include/experimental/bits/simd.h
@@ -2430,17 +2430,23 @@ template
template <>
\
struct __intrinsic_type_impl<_Tp> { using type = __vector _Tp; }
_GLIBCXX_SIMD_PPC_INTRIN(float);
+#ifdef __VSX__
_GLIBCXX_SIMD_PPC_INTRIN(double);
+#endif
_GLIBCXX_SIMD_PPC_INTRIN(signed char);
_GLIBCXX_SIMD_PPC_INTRIN(unsigned char);
_GLIBCXX_SIMD_PPC_INTRIN(signed short);
_GLIBCXX_SIMD_PPC_INTRIN(unsigned short);
_GLIBCXX_SIMD_PPC_INTRIN(signed int);
_GLIBCXX_SIMD_PPC_INTRIN(unsigned int);
+#if defined __VSX__ || __LONG_WIDTH__ == 32
_GLIBCXX_SIMD_PPC_INTRIN(signed long);
_GLIBCXX_SIMD_PPC_INTRIN(unsigned long);
+#endif
+#ifdef __VSX__
_GLIBCXX_SIMD_PPC_INTRIN(signed long long);
_GLIBCXX_SIMD_PPC_INTRIN(unsigned long long);
+#endif
#undef _GLIBCXX_SIMD_PPC_INTRIN
template
@@ -2450,10 +2456,11 @@ template
static constexpr bool _S_is_ldouble = is_same_v<_Tp, long double>;
// allow _Tp == long double with -mlong-double-64
static_assert(!(_S_is_ldouble && sizeof(long double) > sizeof(double)),
- "no __intrinsic_type support for long double on PPC");
+ "no __intrinsic_type support for 128-bit floating point on
PPC");
#ifndef __VSX__
-static_assert(!is_same_v<_Tp, double>,
- "no __intrinsic_type support for double on PPC w/o VSX");
+static_assert(!(is_same_v<_Tp, double>
+ || (_S_is_ldouble && sizeof(long double) ==
sizeof(double))),
+ "no __intrinsic_type support for 64-bit floating point on PPC
w/o VSX");
#endif
using type =
typename __intrinsic_type_impl<
--
Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
Free Software Activist GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts. Ask me about
Ping #5: [PATCH, V4] Eliminate power8 fusion options, use power8 tuning, PR target/102059
Ping #5: | Date: Tue, 12 Apr 2022 21:14:55 -0400 | From: Michael Meissner | Subject: [PATCH, V4] Eliminate power8 fusion options, use power8 tuning, PR target/102059 | Message-ID: https://gcc.gnu.org/pipermail/gcc-patches/2022-April/593153.html We really need closure on this so I can do the backport to GCC 10 that the customer is asking for. -- Michael Meissner, IBM PO Box 98, Ayer, Massachusetts, USA, 01432 email: meiss...@linux.ibm.com
[pushed] c++: improve template-id location
On PR102629 I noticed that we were giving the entire lambda as the location
for this template-id.
Tested x86_64-pc-linux-gnu, applying to trunk.
gcc/cp/ChangeLog:
* pt.cc (tsubst_copy_and_build) [TEMPLATE_ID_EXPR]: Copy location.
(do_auto_deduction): Use expr location.
gcc/testsuite/ChangeLog:
* g++.dg/cpp2a/lambda-pack-init7.C: Check column number.
---
gcc/cp/pt.cc | 28 +++
.../g++.dg/cpp2a/lambda-pack-init7.C | 2 +-
2 files changed, 17 insertions(+), 13 deletions(-)
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 23fbd8245d4..3edee50924f 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -20091,6 +20091,7 @@ tsubst_copy_and_build (tree t,
object = NULL_TREE;
tree tid = lookup_template_function (templ, targs);
+ protected_set_expr_location (tid, EXPR_LOCATION (t));
if (object)
RETURN (build3 (COMPONENT_REF, TREE_TYPE (tid),
@@ -30181,6 +30182,8 @@ do_auto_deduction (tree type, tree init, tree auto_node,
/* Nothing we can do with this, even in deduction context. */
return type;
+ location_t loc = cp_expr_loc_or_input_loc (init);
+
/* [dcl.spec.auto]: Obtain P from T by replacing the occurrences of auto
with either a new invented type template parameter U or, if the
initializer is a braced-init-list (8.5.4), with
@@ -30195,9 +30198,9 @@ do_auto_deduction (tree type, tree init, tree auto_node,
{
if (complain & tf_warning_or_error)
{
- if (permerror (input_location, "direct-list-initialization of "
+ if (permerror (loc, "direct-list-initialization of "
"% requires exactly one element"))
- inform (input_location,
+ inform (loc,
"for deduction to %, use copy-"
"list-initialization (i.e. add %<=%> before the
%<{%>)");
}
@@ -30288,9 +30291,10 @@ do_auto_deduction (tree type, tree init, tree
auto_node,
&& (auto_node
== DECL_SAVED_AUTO_RETURN_TYPE (current_function_decl))
&& LAMBDA_FUNCTION_P (current_function_decl))
- error ("unable to deduce lambda return type from %qE", init);
+ error_at (loc, "unable to deduce lambda return type from %qE",
+ init);
else
- error ("unable to deduce %qT from %qE", type, init);
+ error_at (loc, "unable to deduce %qT from %qE", type, init);
type_unification_real (tparms, targs, parms, , 1, 0,
DEDUCE_CALL,
NULL, /*explain_p=*/true);
@@ -30362,23 +30366,23 @@ do_auto_deduction (tree type, tree init, tree
auto_node,
{
case adc_unspecified:
case adc_unify:
- error("placeholder constraints not satisfied");
+ error_at (loc, "placeholder constraints not satisfied");
break;
case adc_variable_type:
case adc_decomp_type:
- error ("deduced initializer does not satisfy "
-"placeholder constraints");
+ error_at (loc, "deduced initializer does not satisfy "
+ "placeholder constraints");
break;
case adc_return_type:
- error ("deduced return type does not satisfy "
-"placeholder constraints");
+ error_at (loc, "deduced return type does not satisfy "
+ "placeholder constraints");
break;
case adc_requirement:
- error ("deduced expression type does not satisfy "
-"placeholder constraints");
+ error_at (loc, "deduced expression type does not satisfy "
+ "placeholder constraints");
break;
}
- diagnose_constraints (input_location, auto_node, full_targs);
+ diagnose_constraints (loc, auto_node, full_targs);
}
return error_mark_node;
}
diff --git a/gcc/testsuite/g++.dg/cpp2a/lambda-pack-init7.C
b/gcc/testsuite/g++.dg/cpp2a/lambda-pack-init7.C
index f3c3899e97a..face7258c2e 100644
--- a/gcc/testsuite/g++.dg/cpp2a/lambda-pack-init7.C
+++ b/gcc/testsuite/g++.dg/cpp2a/lambda-pack-init7.C
@@ -8,7 +8,7 @@ struct S {};
template
void foo(Args&&... args) {
- [...args = forward /*(args)*/] { // { dg-error "" }
+ [...args = forward /*(args)*/] { // { dg-error "14:" }
[](auto...) { } (forward(args)...);
};
}
base-commit: dcb4bd0789d13dd4d07428bff712d01d3ea71ebe
--
2.27.0
Re: [PATCH] libstdc++: ppc: conditionalize vsx-only simd intrinsics
Hi! On Thu, Apr 28, 2022 at 10:53:40PM -0300, Alexandre Oliva wrote: > On Apr 28, 2022, Segher Boessenkool wrote: > > On Thu, Apr 28, 2022 at 03:09:54AM -0300, Alexandre Oliva wrote: > >> +"no __intrinsic_type support for [long] double on PPC w/o > >> VSX"); > > This change isn't in the changelog. > > It is, it's just that long double is dealt with differently from the > other types, so requiring VSX for it takes a different form: It changes a diagnostic message. Incorrectly, even. The changelog does not talk about this. The only thing I use changelogs for nowadays is to help reviewing code. So when things are missing it stands out like a sore thumb. > > * include/experimental/bits/simd.h [__ALTIVEC__]: Require VSX > > for double, long long, and 64-bit long and 64-bit long double > > intrinsic types. ^^ Yes, that does not talk about changing a diagnostic message. > > The message should not say "long > > double" without qualifying it as "64-bit long double". > > How about qualifying both such messages as in this incremental patchlet: Send full patches always please. Not patches relative to some non-existing state :-) Thanks, Segher
[pushed] c++: also note name used in enclosing class
While looking at PR96645 I noticed that while we were diagnosing names
changing meaning in the full class context, we weren't doing this for
lookups in nested class bodies.
Note that this breaks current range-v3; I've submitted a pull request to fix
its violation of the rule.
Tested x86_64-pc-linux-gnu, applying to trunk.
gcc/cp/ChangeLog:
* class.cc (maybe_note_name_used_in_class): Note in all enclosing
classes. Remember location of use.
(note_name_declared_in_class): Adjust.
gcc/testsuite/ChangeLog:
* g++.dg/lookup/name-clash13.C: New test.
* g++.dg/lookup/name-clash14.C: New test.
* g++.dg/lookup/name-clash15.C: New test.
* g++.dg/lookup/name-clash16.C: New test.
---
gcc/cp/class.cc| 61 +++---
gcc/testsuite/g++.dg/lookup/name-clash13.C | 7 +++
gcc/testsuite/g++.dg/lookup/name-clash14.C | 9
gcc/testsuite/g++.dg/lookup/name-clash15.C | 14 +
gcc/testsuite/g++.dg/lookup/name-clash16.C | 13 +
5 files changed, 86 insertions(+), 18 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/lookup/name-clash13.C
create mode 100644 gcc/testsuite/g++.dg/lookup/name-clash14.C
create mode 100644 gcc/testsuite/g++.dg/lookup/name-clash15.C
create mode 100644 gcc/testsuite/g++.dg/lookup/name-clash16.C
diff --git a/gcc/cp/class.cc b/gcc/cp/class.cc
index bfda0065bf4..bc94ed45e17 100644
--- a/gcc/cp/class.cc
+++ b/gcc/cp/class.cc
@@ -8931,32 +8931,53 @@ is_really_empty_class (tree type, bool ignore_vptr)
void
maybe_note_name_used_in_class (tree name, tree decl)
{
- splay_tree names_used;
-
/* If we're not defining a class, there's nothing to do. */
if (!(innermost_scope_kind() == sk_class
&& TYPE_BEING_DEFINED (current_class_type)
&& !LAMBDA_TYPE_P (current_class_type)))
return;
- /* If there's already a binding for this NAME, then we don't have
- anything to worry about. */
- if (lookup_member (current_class_type, name,
-/*protect=*/0, /*want_type=*/false, tf_warning_or_error))
-return;
+ const cp_binding_level *blev = nullptr;
+ if (const cxx_binding *binding = IDENTIFIER_BINDING (name))
+blev = binding->scope;
+ const cp_binding_level *lev = current_binding_level;
- if (!current_class_stack[current_class_depth - 1].names_used)
-current_class_stack[current_class_depth - 1].names_used
- = splay_tree_new (splay_tree_compare_pointers, 0, 0);
- names_used = current_class_stack[current_class_depth - 1].names_used;
+ /* Record the binding in the names_used tables for classes inside blev. */
+ for (int i = current_class_depth; i > 0; --i)
+{
+ tree type = (i == current_class_depth
+ ? current_class_type
+ : current_class_stack[i].type);
- splay_tree_insert (names_used,
-(splay_tree_key) name,
-(splay_tree_value) decl);
+ for (; lev; lev = lev->level_chain)
+ {
+ if (lev == blev)
+ /* We found the declaration. */
+ return;
+ if (lev->kind == sk_class && lev->this_entity == type)
+ /* This class is inside the declaration scope. */
+ break;
+ }
+
+ auto _used = current_class_stack[i-1].names_used;
+ if (!names_used)
+ names_used = splay_tree_new (splay_tree_compare_pointers, 0, 0);
+
+ tree use = build1_loc (input_location, VIEW_CONVERT_EXPR,
+TREE_TYPE (decl), decl);
+ EXPR_LOCATION_WRAPPER_P (use) = 1;
+ splay_tree_insert (names_used,
+(splay_tree_key) name,
+(splay_tree_value) use);
+}
}
/* Note that NAME was declared (as DECL) in the current class. Check
- to see that the declaration is valid. */
+ to see that the declaration is valid under [class.member.lookup]:
+
+ If [the result of a search in T for N at point P] differs from the result of
+ a search in T for N from immediately after the class-specifier of T, the
+ program is ill-formed, no diagnostic required. */
void
note_name_declared_in_class (tree name, tree decl)
@@ -8979,6 +9000,9 @@ note_name_declared_in_class (tree name, tree decl)
n = splay_tree_lookup (names_used, (splay_tree_key) name);
if (n)
{
+ tree use = (tree) n->value;
+ location_t loc = EXPR_LOCATION (use);
+ tree olddecl = OVL_FIRST (TREE_OPERAND (use, 0));
/* [basic.scope.class]
A name N used in a class S shall refer to the same declaration
@@ -8987,9 +9011,10 @@ note_name_declared_in_class (tree name, tree decl)
if (permerror (location_of (decl),
"declaration of %q#D changes meaning of %qD",
decl, OVL_NAME (decl)))
- inform (location_of ((tree) n->value),
- "%qD declared here as %q#D",
- OVL_NAME (decl), (tree) n->value);
+ {
+ inform (loc, "used here to mean
Re: [PATCH] ipa-visibility: Optimize TLS access [PR99619]
On 5/2/22 18:04, Martin Jambor wrote: > (Minor nit and I don't care too much, but in GCC we traditionally > specify co-authors in the ChangeLog entry beginning by providing more > names, one per line. But perhaps we want to adapt more widely used > practices.) Hello. Using Co-Authored-By is fine and actually preferred over the traditional listing of multiple authors. Moreover, mentioning date and author in a changelog entry is not necessary, both can be taken from git commit by our changelog hooks. Cheers, Martin
[PATCH] c++: partial spec constraint checking context [PR105220]
Currently when checking the constraints of a class template, we do so in
the context of the template, not the specialized type. This is the best
we can do for a primary template since the specialized type is valid
only if the primary template's constraints are satisfied. But for a
partial specialization, we can assume the specialized type is valid (as
a consequence of constraints being checked only when necessary), so we
arguably should check the constraints on a partial specialization more
specifically in the context of the specialized type, not the template.
This patch implements this by substituting and setting the access
context appropriately in satisfy_declaration_constraints. Note that
setting the access context in this case is somewhat redundant since the
relevant caller most_specialized_partial_spec will already have set the
access context to the specialiation, but this redundancy should be harmless.
Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk and perhaps 12.2 (after the branch is thawed)?
PR c++/105220
gcc/cp/ChangeLog:
* constraint.cc (satisfy_declaration_constraints): When checking
the constraints of a partial template specialization, do so in
the context of the specialized type not the template.
gcc/testsuite/ChangeLog:
* g++.dg/cpp2a/concepts-partial-spec12.C: New test.
---
gcc/cp/constraint.cc | 17 ++---
.../g++.dg/cpp2a/concepts-partial-spec12.C| 19 +++
2 files changed, 33 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec12.C
diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index 94f6222b436..772f8532b47 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -3253,11 +3253,22 @@ satisfy_declaration_constraints (tree t, tree args,
sat_info info)
{
if (!push_tinst_level (t, args))
return result;
- tree pattern = DECL_TEMPLATE_RESULT (t);
+ tree ascope = DECL_TEMPLATE_RESULT (t);
+ if (CLASS_TYPE_P (TREE_TYPE (t))
+ && CLASSTYPE_TEMPLATE_SPECIALIZATION (TREE_TYPE (t)))
+ {
+ gcc_checking_assert (t == most_general_template (t));
+ /* When checking the constraints on a partial specialization,
+do so in the context of the specialized type, not the template.
+This substitution should always succeed since we shouldn't
+be checking constraints thereof unless the specialized type
+is valid. */
+ ascope = tsubst (ascope, args, tf_none, info.in_decl);
+ }
push_to_top_level ();
- push_access_scope (pattern);
+ push_access_scope (ascope);
result = satisfy_normalized_constraints (norm, args, info);
- pop_access_scope (pattern);
+ pop_access_scope (ascope);
pop_from_top_level ();
pop_tinst_level ();
}
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec12.C
b/gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec12.C
new file mode 100644
index 000..641d456722d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec12.C
@@ -0,0 +1,19 @@
+// PR c++/105220
+// { dg-do compile { target c++20 } }
+
+template
+concept fooable = requires(T t) { t.foo(); };
+
+template
+struct A;// #1, incomplete
+
+template
+struct A { }; // #2
+
+struct B {
+private:
+ friend struct A;
+ void foo();
+};
+
+template struct A; // OK, B::foo() is accessible from #2
--
2.36.0.44.g0f828332d5
Re: [PATCH] x86: Add missing .note.GNU-stack to assembly source
On Fri, Apr 29, 2022 at 10:38 AM H.J. Lu wrote: > > Add .note.GNU-stack assembly source to avoid linker warning: > > ld: warning: /tmp/ccPZSZ7Z.o: missing .note.GNU-stack section implies > executable stack > ld: NOTE: This behaviour is deprecated and will be removed in a future > version of the linker > FAIL: gcc.target/i386/iamcu/test_3_element_struct_and_unions.c compilation, > -O0 > > PR testsuite/105433 > * gcc.target/i386/iamcu/asm-support.S: Add .note.GNU-stack. > * gcc.target/x86_64/abi/asm-support.S: Likewise. > * gcc.target/x86_64/abi/avx/asm-support.S: Likewise. > * gcc.target/x86_64/abi/avx512f/asm-support.S: Likewise. > * gcc.target/x86_64/abi/avx512fp16/asm-support.S: Likewise. > * gcc.target/x86_64/abi/avx512fp16/m256h/asm-support.S: Likewise. > * gcc.target/x86_64/abi/avx512fp16/m512h/asm-support.S: Likewise. > * gcc.target/x86_64/abi/ms-sysv/do-test.S: Likewise. > --- > gcc/testsuite/gcc.target/i386/iamcu/asm-support.S| 1 + > gcc/testsuite/gcc.target/x86_64/abi/asm-support.S| 1 + > gcc/testsuite/gcc.target/x86_64/abi/avx/asm-support.S| 1 + > gcc/testsuite/gcc.target/x86_64/abi/avx512f/asm-support.S| 1 + > gcc/testsuite/gcc.target/x86_64/abi/avx512fp16/asm-support.S | 1 + > .../gcc.target/x86_64/abi/avx512fp16/m256h/asm-support.S | 1 + > .../gcc.target/x86_64/abi/avx512fp16/m512h/asm-support.S | 1 + > gcc/testsuite/gcc.target/x86_64/abi/ms-sysv/do-test.S| 1 + > 8 files changed, 8 insertions(+) > > diff --git a/gcc/testsuite/gcc.target/i386/iamcu/asm-support.S > b/gcc/testsuite/gcc.target/i386/iamcu/asm-support.S > index b4a4a140e54..db08f52a34f 100644 > --- a/gcc/testsuite/gcc.target/i386/iamcu/asm-support.S > +++ b/gcc/testsuite/gcc.target/i386/iamcu/asm-support.S > @@ -300,3 +300,4 @@ iamcu_noprintf: > .align 4 > .LCiamcu_noprintf1: > .long 1132527616 > + .section.note.GNU-stack,"",@progbits > diff --git a/gcc/testsuite/gcc.target/x86_64/abi/asm-support.S > b/gcc/testsuite/gcc.target/x86_64/abi/asm-support.S > index 7a8ed03d119..2f8d3a09c6b 100644 > --- a/gcc/testsuite/gcc.target/x86_64/abi/asm-support.S > +++ b/gcc/testsuite/gcc.target/x86_64/abi/asm-support.S > @@ -82,3 +82,4 @@ snapshot_ret: > .comm xmm_regs,256,32 > .comm x87_regs,128,32 > .comm volatile_var,8,8 > + .section.note.GNU-stack,"",@progbits > diff --git a/gcc/testsuite/gcc.target/x86_64/abi/avx/asm-support.S > b/gcc/testsuite/gcc.target/x86_64/abi/avx/asm-support.S > index 73a59191d6d..77b3480ac32 100644 > --- a/gcc/testsuite/gcc.target/x86_64/abi/avx/asm-support.S > +++ b/gcc/testsuite/gcc.target/x86_64/abi/avx/asm-support.S > @@ -79,3 +79,4 @@ snapshot_ret: > .comm ymm_regs,512,32 > .comm x87_regs,128,32 > .comm volatile_var,8,8 > + .section.note.GNU-stack,"",@progbits > diff --git a/gcc/testsuite/gcc.target/x86_64/abi/avx512f/asm-support.S > b/gcc/testsuite/gcc.target/x86_64/abi/avx512f/asm-support.S > index 0ef82876dd9..2e3306c44cb 100644 > --- a/gcc/testsuite/gcc.target/x86_64/abi/avx512f/asm-support.S > +++ b/gcc/testsuite/gcc.target/x86_64/abi/avx512f/asm-support.S > @@ -95,3 +95,4 @@ snapshot_ret: > .comm zmm_regs,2048,64 > .comm x87_regs,128,32 > .comm volatile_var,8,8 > + .section.note.GNU-stack,"",@progbits > diff --git a/gcc/testsuite/gcc.target/x86_64/abi/avx512fp16/asm-support.S > b/gcc/testsuite/gcc.target/x86_64/abi/avx512fp16/asm-support.S > index 7849acd2649..0793acf048b 100644 > --- a/gcc/testsuite/gcc.target/x86_64/abi/avx512fp16/asm-support.S > +++ b/gcc/testsuite/gcc.target/x86_64/abi/avx512fp16/asm-support.S > @@ -79,3 +79,4 @@ snapshot_ret: > .comm xmm_regs,256,32 > .comm x87_regs,128,32 > .comm volatile_var,8,8 > + .section.note.GNU-stack,"",@progbits > diff --git > a/gcc/testsuite/gcc.target/x86_64/abi/avx512fp16/m256h/asm-support.S > b/gcc/testsuite/gcc.target/x86_64/abi/avx512fp16/m256h/asm-support.S > index 73a59191d6d..77b3480ac32 100644 > --- a/gcc/testsuite/gcc.target/x86_64/abi/avx512fp16/m256h/asm-support.S > +++ b/gcc/testsuite/gcc.target/x86_64/abi/avx512fp16/m256h/asm-support.S > @@ -79,3 +79,4 @@ snapshot_ret: > .comm ymm_regs,512,32 > .comm x87_regs,128,32 > .comm volatile_var,8,8 > + .section.note.GNU-stack,"",@progbits > diff --git > a/gcc/testsuite/gcc.target/x86_64/abi/avx512fp16/m512h/asm-support.S > b/gcc/testsuite/gcc.target/x86_64/abi/avx512fp16/m512h/asm-support.S > index 0ef82876dd9..2e3306c44cb 100644 > --- a/gcc/testsuite/gcc.target/x86_64/abi/avx512fp16/m512h/asm-support.S > +++ b/gcc/testsuite/gcc.target/x86_64/abi/avx512fp16/m512h/asm-support.S > @@ -95,3 +95,4 @@ snapshot_ret: > .comm zmm_regs,2048,64 > .comm x87_regs,128,32
[ping2][PATCH 0/8][RFC] Support BTF decl_tag and type_tag annotations
Pinging this series again.
Link: https://gcc.gnu.org/pipermail/gcc-patches/2022-April/592685.html
This series adds new C-family frontend attributes for recording string
"tags" in DWARF and BTF debug info to support kernel use cases.
There remains one issue in the implementation which has not been
resolved, which I hope someone in the GCC community may be able to shed
some light on.
Specifically, it is related to how GCC parses the attributes: In cases
where the new btf_type_tag attribute (which applies to types) is
specified multiple times on different intermediate pointer types of a
declaration, GCC seems to attach the attributes to the TREEs in an
unexpected order.
As a result the behavior of the attribute in GCC cannot be reconciled
with its definition in BTF or behavior in the clang compiler.
Consider the following example:
#define __typetag1 __attribute__((btf_type_tag("tag1")))
#define __typetag2 __attribute__((btf_type_tag("tag2")))
#define __typetag3 __attribute__((btf_type_tag("tag3")))
int __typetag1 * __typetag2 __typetag3 * g;
The expected behavior is that 'g' is "a pointer with tags 'tag2' and
'tag3', to a pointer with tag 'tag1' to an int". i.e.:
>>>
But GCC's attribute parsing produces a variable 'g' which is "a pointer
with tag 'tag1' to a pointer with tags 'tag2' and 'tag3' to an int", i.e.
>>>
And as a result the DWARF and BTF generated for cases like this do not
agree with the BTF type tag specification nor output of the clang
compiler, which already supports this feature.
(Please refer to the "Current issues in implementation" section in the
series cover letter for the full details of this example.)
So far I have been unable to resolve this issue in the btf_type_tag
attribute handler. It seems to me that the cause must be "higher up" in
the C frontend attribute parsing but I am not familiar with this area of
GCC.
Any insight into understanding this issue or comments elsewhere in the
series would be most welcome.
Thanks,
David
On 4/18/22 12:36, David Faust via Gcc-patches wrote:
Gentle ping :)
Link: https://gcc.gnu.org/pipermail/gcc-patches/2022-April/592685.html
The series adds support for new attribues btf_type_tag and btf_decl_tag,
for recording arbitrary string tags in DWARF and BTF debug info. The
feature is to support kernel use cases.
Thanks,
David
On 4/1/22 12:42, David Faust via Gcc-patches wrote:
Hello,
This patch series is a first attempt at adding support for:
- Two new C-language-level attributes that allow to associate (to "tag")
particular declarations and types with arbitrary strings. As explained
below,
this is intended to be used to, for example, characterize certain pointer
types.
- The conveyance of that information in the DWARF output in the form of a new
DIE: DW_TAG_GNU_annotation.
- The conveyance of that information in the BTF output in the form of two new
kinds of BTF objects: BTF_KIND_DECL_TAG and BTF_KIND_TYPE_TAG.
All of these facilities are being added to the eBPF ecosystem, and support for
them exists in some form in LLVM. However, as we shall see, we have found some
problems implementing them so some discussion is in order.
Purpose
===
1) Addition of C-family language constructs (attributes) to specify free-text
tags on certain language elements, such as struct fields.
The purpose of these annotations is to provide additional information
about
types, variables, and function paratemeters of interest to the kernel. A
driving use case is to tag pointer types within the linux kernel and eBPF
programs with additional semantic information, such as '__user' or
'__rcu'.
For example, consider the linux kernel function do_execve with the
following declaration:
static int do_execve(struct filename *filename,
const char __user *const __user *__argv,
const char __user *const __user *__envp);
Here, __user could be defined with these annotations to record semantic
information about the pointer parameters (e.g., they are user-provided) in
DWARF and BTF information. Other kernel facilites such as the eBPF
verifier
can read the tags and make use of the information.
2) Conveying the tags in the generated DWARF debug info.
The main motivation for emitting the tags in DWARF is that the Linux
kernel
generates its BTF information via pahole, using DWARF as a source:
++ BTF BTF +--+
| pahole |---> vmlinux.btf --->| verifier |
++ +--+
^^
||
DWARF |BTF |
||
vmlinux
Re: [PATCH] ipa-visibility: Optimize TLS access [PR99619]
On Mon, 2 May 2022, Martin Jambor wrote:
> > Co-Authored-By: Alexander Monakov
>
> (Minor nit and I don't care too much, but in GCC we traditionally
> specify co-authors in the ChangeLog entry beginning by providing more
> names, one per line. But perhaps we want to adapt more widely used
> practices.)
I believe this is the recommended way to specify co-authors now (after
Git migration) according to https://gcc.gnu.org/codingconventions.html
(patch discussion below)
> > --- a/gcc/ipa-visibility.cc
> > +++ b/gcc/ipa-visibility.cc
> > @@ -872,6 +872,22 @@ function_and_variable_visibility (bool whole_program)
> > }
> > }
> > }
> > + FOR_EACH_VARIABLE (vnode)
> > +{
> > + tree decl = vnode->decl;
> > +
> > + /* Optimize TLS model based on visibility (taking into account
> > + optimizations done in the preceding loop), unless it was
> > + specified explicitly. */
> > +
> > + if (DECL_THREAD_LOCAL_P (decl)
> > + && !lookup_attribute ("tls_model", DECL_ATTRIBUTES (decl)))
> > +{
> > + enum tls_model new_model = decl_default_tls_model (decl);
> > + gcc_checking_assert (new_model >= decl_tls_model (decl));
> > + set_decl_tls_model (decl, new_model);
> > +}
> > +}
> >
>
> decl_default_tls_model depends on the global optimize flag, which is
> almost always problematic in IPA passes. I was able to make your patch
> ICE using the vis-attr-hidden.c testcase from your patch with:
>
> mjambor@virgil:~/gcc/small/tests/tls$ ~/gcc/small/inst/bin/gcc -O2 -fPIC
> -flto -c vis-attr-hidden.c
> mjambor@virgil:~/gcc/small/tests/tls$ ~/gcc/small/inst/bin/gcc -fPIC -O0
> -shared -flto vis-attr-hidden.o
> during IPA pass: whole-program
> lto1: internal compiler error: in function_and_variable_visibility, at
> ipa-visibility.cc:888
[snip]
> Note the use of LTO, mismatching -O flags and the -shared flag in the
> link step.
Ah, right. The assert is checking that we don't accidentally downgrade decl's
TLS access model, e.g. from local-dynamic to global-dynamic, and you've shown
how to trigger that. I didn't realize this code can run twice, and with
different 'optimize' levels.
I would suggest to solve this by checking if the new TLS model is stronger,
i.e. instead of this:
gcc_checking_assert (new_model >= decl_tls_model (decl));
set_decl_tls_model (decl, new_model);
do this:
if (new_model >= decl_tls_model (decl))
set_decl_tls_model (decl, new_model);
Does this look reasonable?
> A simple but somewhat lame way to avoid the ICE would be to run your
> loop over variables only from pass_ipa_function_and_variable_visibility
> and not from pass_ipa_whole_program_visibility.
>
> I am afraid a real solution would involve copying relevant entries from
> global_options to the symtab node representing the variable when it is
> created/finalized, properly streaming them for LTO, and modifying
> decl_default_tls_model to rely on those rather than global_options
> itself.
If we agree on the solution above, then this will not be necessary, after all
this transformation looks at optimized whole-program visibility status,
and so initial optimization level should not be relevant.
> But maybe Honza has some other clever idea.
>
> Also, please be careful not to unnecessarily commit trailing blank
> spaces, the empty lines in your patch are not really empty.
Yep, I can take care of whitespace issues.
Thank you!
Alexander
[PATCH] c++: parse error with >= in template argument list [PR105436]
This patch fixes an oversight whereby we treated >= as the end of a template argument. This causes problems in C++14, because in cp_parser_template_argument we go different ways for C++14 and C++17: /* It must be a non-type argument. In C++17 any constant-expression is allowed. */ if (cxx_dialect > cxx14) goto general_expr; so in this testcase in C++14 we get "N" as the template argument but in C++17 it is the whole "N >= 5" expression. So in C++14 the remaining ">= 5" triggered the newly-added diagnostic. Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? PR c++/105436 gcc/cp/ChangeLog: * parser.cc (cp_parser_next_token_ends_template_argument_p): Don't return true for CPP_GREATER_EQ. gcc/testsuite/ChangeLog: * g++.dg/parse/template31.C: New test. --- gcc/cp/parser.cc| 1 - gcc/testsuite/g++.dg/parse/template31.C | 4 2 files changed, 4 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.dg/parse/template31.C diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc index a5cbb3e896f..5fa743b5a8e 100644 --- a/gcc/cp/parser.cc +++ b/gcc/cp/parser.cc @@ -33224,7 +33224,6 @@ cp_parser_next_token_ends_template_argument_p (cp_parser *parser) || ((cxx_dialect != cxx98) && token->type == CPP_RSHIFT) /* For better diagnostics, treat >>= like that too, that shouldn't appear non-nested in template arguments. */ - || token->type == CPP_GREATER_EQ || token->type == CPP_RSHIFT_EQ); } diff --git a/gcc/testsuite/g++.dg/parse/template31.C b/gcc/testsuite/g++.dg/parse/template31.C new file mode 100644 index 000..a5693e851f7 --- /dev/null +++ b/gcc/testsuite/g++.dg/parse/template31.C @@ -0,0 +1,4 @@ +// PR c++/105436 + +template struct A; +template A= 5> f(); base-commit: 1cb220498e1f59021dab36c39c5d726e9f070c6a -- 2.35.1
[PATCH] c++: wrong parse with functors [PR64679]
Consider
struct F {
F(int) {}
F operator()(int) const { return *this; }
};
and
F(i)(0)(0);
where we're supposed to first call the constructor and then invoke
the operator() twice. However, we parse this as an init-declarator:
"(i)" looks like a perfectly valid declarator, then we see an '(' and
think it must be an initializer, so we commit and we're toast. My
fix is to look a little bit farther before deciding we've seen an
initializer.
This is only a half of c++/64679, the other part of the PR is unrelated:
there the problem is that we are calling pushdecl while parsing
tentatively (in cp_parser_parameter_declaration_list), which is bad.
I don't know how to fix it though, maybe move the pushdecl call to
grokparms? Tricky :(.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
PR c++/64679
gcc/cp/ChangeLog:
* parser.cc (cp_parser_init_declarator): Properly handle a series of
operator() calls, they are not part of an init-declarator.
gcc/testsuite/ChangeLog:
* g++.dg/parse/functor1.C: New test.
---
gcc/cp/parser.cc | 25 -
gcc/testsuite/g++.dg/parse/functor1.C | 22 ++
2 files changed, 46 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/parse/functor1.C
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index a5cbb3e896f..6e2936b68ef 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -22636,11 +22636,34 @@ cp_parser_init_declarator (cp_parser* parser,
return error_mark_node;
}
- /* An `=' or an `(', or an '{' in C++0x, indicates an initializer. */
+ /* An `=' or an '{' in C++11, indicate an initializer. An '(' may indicate
+ an initializer as well. */
if (token->type == CPP_EQ
|| token->type == CPP_OPEN_PAREN
|| token->type == CPP_OPEN_BRACE)
{
+ /* Don't get fooled into thinking that F(i)(1)(2) is an initializer.
+It isn't; it's an expression. (Here '(i)' would have already been
+parsed as a declarator.) */
+ if (token->type == CPP_OPEN_PAREN
+ && cp_parser_uncommitted_to_tentative_parse_p (parser))
+ {
+ cp_lexer_save_tokens (parser->lexer);
+ cp_lexer_consume_token (parser->lexer);
+ cp_parser_skip_to_closing_parenthesis (parser,
+/*recovering*/false,
+/*or_comma*/false,
+/*consume_paren*/true);
+ /* If this is an initializer, only a ',' or ';' can follow: either
+we have another init-declarator, or we're at the end of an
+init-declarator-list which can only be followed by a ';'. */
+ bool ok = (cp_lexer_next_token_is (parser->lexer, CPP_SEMICOLON)
+|| cp_lexer_next_token_is (parser->lexer, CPP_COMMA));
+ cp_lexer_rollback_tokens (parser->lexer);
+ if (__builtin_expect (!ok, 0))
+ /* Not an init-declarator. */
+ return error_mark_node;
+ }
is_initialized = SD_INITIALIZED;
initialization_kind = token->type;
declarator->init_loc = token->location;
diff --git a/gcc/testsuite/g++.dg/parse/functor1.C
b/gcc/testsuite/g++.dg/parse/functor1.C
new file mode 100644
index 000..c014114c098
--- /dev/null
+++ b/gcc/testsuite/g++.dg/parse/functor1.C
@@ -0,0 +1,22 @@
+// PR c++/64679
+// { dg-do run }
+
+struct F {
+ F(int) { }
+ F(int, int) { }
+ F operator()(int) const { return *this; }
+ F operator()(int, int) const { return *this; }
+};
+
+int main()
+{
+ // Init-declarators.
+ int i = 0;
+ int (j)(1);
+ // None of these is an init-declarator.
+ F(i)(1)(2);
+ F(i)(1, 2)(3);
+ F(i)(1)(2, 3);
+ F(i)(2)(3)(4)(5);
+ F(i, j)(1)(2)(3)(4)(5)(6);
+}
base-commit: 1cb220498e1f59021dab36c39c5d726e9f070c6a
--
2.35.1
Re: [PATCH] ipa-visibility: Optimize TLS access [PR99619]
Hello,
On Sun, Apr 17 2022, Artem Klimov via Gcc-patches wrote:
> Fix issue PR99619, which asks to optimize TLS access based on
> visibility. The fix is implemented as an IPA optimization, which allows
> to take optimized visibility status into account (as well as avoid
> modifying all language frontends).
I'm afraid I'll have to defer to Honza to approve this, but I have found
an issue with the patch.
>
> 2022-04-17 Artem Klimov
>
> gcc/ChangeLog:
> PR middle-end/99619
> * ipa-visibility.cc (function_and_variable_visibility): Add an
> explicit TLS model update after visibility optimisation loops.
>
> gcc/testsuite/ChangeLog:
> PR middle-end/99619
> * gcc.dg/tls/vis-attr-gd.c: New test.
> * gcc.dg/tls/vis-attr-hidden-gd.c: New test.
> * gcc.dg/tls/vis-attr-hidden.c: New test.
> * gcc.dg/tls/vis-flag-hidden-gd.c: New test.
> * gcc.dg/tls/vis-flag-hidden.c: New test.
> * gcc.dg/tls/vis-pragma-hidden-gd.c: New test.
> * gcc.dg/tls/vis-pragma-hidden.c: New test.
>
> Co-Authored-By: Alexander Monakov
(Minor nit and I don't care too much, but in GCC we traditionally
specify co-authors in the ChangeLog entry beginning by providing more
names, one per line. But perhaps we want to adapt more widely used
practices.)
> Signed-off-by: Artem Klimov
> ---
[...]
> diff --git a/gcc/ipa-visibility.cc b/gcc/ipa-visibility.cc
> index e95a0dd252f..ca5b9a95f5e 100644
> --- a/gcc/ipa-visibility.cc
> +++ b/gcc/ipa-visibility.cc
> @@ -872,6 +872,22 @@ function_and_variable_visibility (bool whole_program)
> }
> }
> }
> + FOR_EACH_VARIABLE (vnode)
> +{
> + tree decl = vnode->decl;
> +
> + /* Optimize TLS model based on visibility (taking into account
> + optimizations done in the preceding loop), unless it was
> + specified explicitly. */
> +
> + if (DECL_THREAD_LOCAL_P (decl)
> + && !lookup_attribute ("tls_model", DECL_ATTRIBUTES (decl)))
> +{
> + enum tls_model new_model = decl_default_tls_model (decl);
> + gcc_checking_assert (new_model >= decl_tls_model (decl));
> + set_decl_tls_model (decl, new_model);
> +}
> +}
>
decl_default_tls_model depends on the global optimize flag, which is
almost always problematic in IPA passes. I was able to make your patch
ICE using the vis-attr-hidden.c testcase from your patch with:
mjambor@virgil:~/gcc/small/tests/tls$ ~/gcc/small/inst/bin/gcc -O2 -fPIC
-flto -c vis-attr-hidden.c
mjambor@virgil:~/gcc/small/tests/tls$ ~/gcc/small/inst/bin/gcc -fPIC -O0
-shared -flto vis-attr-hidden.o
during IPA pass: whole-program
lto1: internal compiler error: in function_and_variable_visibility, at
ipa-visibility.cc:888
0x25f48c0 function_and_variable_visibility
/home/mjambor/gcc/small/src/gcc/ipa-visibility.cc:888
0x25f4b33 whole_program_function_and_variable_visibility
/home/mjambor/gcc/small/src/gcc/ipa-visibility.cc:938
0x25f4bda execute
/home/mjambor/gcc/small/src/gcc/ipa-visibility.cc:986
Please submit a full bug report, with preprocessed source (by using
-freport-bug).
Please include the complete backtrace with any bug report.
See
Make 'c-c++-common/goacc/kernels-decompose-pr100400-1-*.c' behave consistently, regardless of checking level (was: GCC 12.1 Release Candidate available from gcc.gnu.org)
Hi!
On 2022-05-01T11:02:29+0100, Iain Sandoe via Gcc wrote:
>> On 29 Apr 2022, at 15:34, Jakub Jelinek via Gcc wrote:
>>
>> The first release candidate for GCC 12.1 is available from
>>
>> https://gcc.gnu.org/pub/gcc/snapshots/12.1.0-RC-20220429/
>> ftp://gcc.gnu.org/pub/gcc/snapshots/12.1.0-RC-20220429/
>>
>> and shortly its mirrors. It has been generated from git commit
>> r12-8321-g621650f64fb667.
> [...] new fails (presumably because checking is off):
> XPASS: c-c++-common/goacc/kernels-decompose-pr100400-1-2.c -std=c++98
> (internal compiler error)
> FAIL: c-c++-common/goacc/kernels-decompose-pr100400-1-2.c -std=c++98 (test
> for excess errors)
> XPASS: c-c++-common/goacc/kernels-decompose-pr100400-1-2.c -std=c++14
> (internal compiler error)
> FAIL: c-c++-common/goacc/kernels-decompose-pr100400-1-2.c -std=c++14 (test
> for excess errors)
> XPASS: c-c++-common/goacc/kernels-decompose-pr100400-1-2.c -std=c++17
> (internal compiler error)
> FAIL: c-c++-common/goacc/kernels-decompose-pr100400-1-2.c -std=c++17 (test
> for excess errors)
> XPASS: c-c++-common/goacc/kernels-decompose-pr100400-1-2.c -std=c++20
> (internal compiler error)
> FAIL: c-c++-common/goacc/kernels-decompose-pr100400-1-2.c -std=c++20 (test
> for excess errors)
Confirmed, and sorry. I had taken care to add explicit '-fchecking'
next to 'dg-ice', but that's in fact not the problem/cure here.
OK to push to the relevant branches the attached
"Make 'c-c++-common/goacc/kernels-decompose-pr100400-1-*.c' behave
consistently, regardless of checking level"?
Grüße
Thomas
-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht
München, HRB 106955
>From 7827d018a9e0109b8d271ceb824f2c718bae508e Mon Sep 17 00:00:00 2001
From: Thomas Schwinge
Date: Mon, 2 May 2022 15:15:26 +0200
Subject: [PATCH] Make 'c-c++-common/goacc/kernels-decompose-pr100400-1-*.c'
behave consistently, regardless of checking level
Fix-up for commit c14ea6a72fb1ae66e3d32ac8329558497c6e4403
"Catch 'GIMPLE_DEBUG' misbehavior in OpenACC 'kernels' decomposition
[PR100400, PR103836, PR104061]".
For C++ compilation of 'c-c++-common/goacc/kernels-decompose-pr100400-1-2.c',
we first emit a 'sorry' diagnostic, and then a 'gcc_unreachable' (or 'abort',
see below) diagnostic, but for example, for '--enable-checking=release' (thus,
'!CHECKING_P'), the second one may actually be turned into a
'confused by earlier errors, bailing out' diagnostic. (See
'gcc/diagnostic.cc:diagnostic_report_diagnostic': "When not checking, ICEs are
converted to fatal errors when an error has already occurred.") Thus, make
'c-c++-common/goacc/kernels-decompose-pr100400-1-2.c' behave consistently via
'-Wfatal-errors', and thus only matching the 'sorry' diagnostic.
For example, for '--enable-checking=no' (thus, '!ENABLE_ASSERT_CHECKING'), a
call to 'gcc_unreachable' cannot be assumed emit an 'abort'-like diagnostic, so
explicitly call 'abort' in
'gcc/omp-oacc-kernels-decompose.cc:visit_loops_in_gang_single_region', in the
'GIMPLE_OMP_FOR' case, to avoid regressing
'c-c++-common/goacc/kernels-decompose-pr100400-1-3.c', and
'c-c++-common/goacc/kernels-decompose-pr100400-1-4.c'.
PR middle-end/100400
gcc/
* omp-oacc-kernels-decompose.cc
(visit_loops_in_gang_single_region) : Reliably
'abort'.
gcc/testsuite/
* c-c++-common/goacc/kernels-decompose-pr100400-1-2.c: Specify
'-Wfatal-errors'.
---
gcc/omp-oacc-kernels-decompose.cc| 6 ++
.../goacc/kernels-decompose-pr100400-1-2.c | 12 +---
2 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/gcc/omp-oacc-kernels-decompose.cc b/gcc/omp-oacc-kernels-decompose.cc
index 4386787ba3c..9f5c1ecb255 100644
--- a/gcc/omp-oacc-kernels-decompose.cc
+++ b/gcc/omp-oacc-kernels-decompose.cc
@@ -239,7 +239,13 @@ visit_loops_in_gang_single_region (gimple_stmt_iterator *gsi_p,
case GIMPLE_OMP_FOR:
/*TODO Given the current 'adjust_region_code' algorithm, this is
actually... */
+#if 0
gcc_unreachable ();
+#else
+ /* ..., but due to bugs (PR100400), we may actually come here.
+ Reliably catch this, regardless of checking level. */
+ abort ();
+#endif
{
tree clauses = gimple_omp_for_clauses (stmt);
diff --git a/gcc/testsuite/c-c++-common/goacc/kernels-decompose-pr100400-1-2.c b/gcc/testsuite/c-c++-common/goacc/kernels-decompose-pr100400-1-2.c
index a643f109bf1..8b65e07c623 100644
--- a/gcc/testsuite/c-c++-common/goacc/kernels-decompose-pr100400-1-2.c
+++ b/gcc/testsuite/c-c++-common/goacc/kernels-decompose-pr100400-1-2.c
@@ -1,8 +1,8 @@
/* { dg-additional-options "--param openacc-kernels=decompose" } */
-/* { dg-additional-options "-fchecking" }
- { dg-ice TODO { c++ } }
- { dg-prune-output "during GIMPLE pass: omp_oacc_kernels_decompose" } */
+/* Ensure consistent
[PATCH] Assume a call is expensive when it mismatches
This makes sure to not consider calls to builtin decls with mismatching arguments as inexpensive. Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed. 2022-04-13 Richard Biener * tree-scalar-evolution.cc (expression_expensive_p): Never consider mismatched calls as cheap. --- gcc/tree-scalar-evolution.cc | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/gcc/tree-scalar-evolution.cc b/gcc/tree-scalar-evolution.cc index 44157265ce8..b53d7aaa71d 100644 --- a/gcc/tree-scalar-evolution.cc +++ b/gcc/tree-scalar-evolution.cc @@ -3420,12 +3420,15 @@ expression_expensive_p (tree expr, hash_map , break; return true; } + break; + default: + if (cfn == CFN_LAST + || !is_inexpensive_builtin (get_callee_fndecl (expr))) + return true; break; } - if (!is_inexpensive_builtin (get_callee_fndecl (expr))) - return true; FOR_EACH_CALL_EXPR_ARG (arg, iter, expr) if (expression_expensive_p (arg, cache, op_cost)) return true; -- 2.34.1
Re: [PATCH] PR tree-optimization/102950: Improved EVRP for signed BIT_XOR_EXPR.
On Tue, Feb 1, 2022 at 9:57 AM Roger Sayle wrote: > > > This patch fixes PR tree-optimization/102950, which is a P2 regression, > by providing better range bounds for BIT_XOR_EXPR, BIT_AND_EXPR and > BIT_IOR_EXPR on signed integer types. In general terms, any binary > bitwise operation on sign-extended or zero-extended integer types will > produce results that are themselves sign-extended or zero-extended. > More precisely, we can derive signed bounds from the number of leading > redundant sign bit copies, from the equation: > clrsb(X op Y) >= min (clrsb (X), clrsb(Y)) > and from the property that for any (signed or unsigned) range [lb, ub] > that clrsb([lb, ub]) >= min (clrsb(lb), clrsb(ub)). > > These can be used to show that [-1, 0] op [-1, 0] is [-1, 0] or that > [-128, 127] op [-128, 127] is [-128, 127], even when tracking nonzero > bits would result in VARYING (as every bit can be 0 or 1). This is > equivalent to determining the minimum type precision in which the > operation can be performed then sign extending the result. > > One additional refinement is to observe that X ^ Y can never be > zero if the ranges of X and Y don't overlap, i.e. X can't be equal > to Y. > > Previously, the expression "(int)(char)a ^ 233" in the PR was considered > VARYING, but with the above changes now has the range [-256, -1][1, 255], > which is sufficient to optimize away the call to foo. > > > This patch has been tested on x86_64-pc-linux-gnu with make bootstrap > and make -k check with no new failures. Ok for mainline? OK for trunk. Thanks, Richard. > > 2022-02-01 Roger Sayle > > gcc/ChangeLog > PR tree-optimization/102950 > * range-op.cc (wi_optimize_signed_bitwise_op): New function to > determine bounds of bitwise operations on signed types. > (operator_bitwise_and::wi_fold): Call the above function. > (operator_bitwise_or::wi_fold): Likewise. > (operator_bitwise_xor::wi_fold): Likewise. Additionally, the > result can't be zero if the operands can't be equal. > > gcc/testsuite/ChangeLog > PR tree-optimization/102950 > gcc.dg/pr102950.c: New test case. > gcc.dg/tree-ssa/evrp10.c: New test case. > > > Thanks in advance (and Happy Chinese New Year), > Roger > -- >
Re: [PATCH] PR middle-end/98865: Optimize (a>>63)*b as -(a>>63) in match.pd.
On Wed, Apr 20, 2022 at 8:35 PM Roger Sayle wrote:
>
>
> This patch implements the constant folding optimization(s) described in
> PR middle-end/98865, which should help address the serious performance
> regression of Botan AES-128/XTS mentioned in PR tree-optimization/98856.
> This combines aspects of both Jakub Jelinek's patch in comment #2 and
> Andrew Pinski's patch in comment #4, so both are listed as co-authors.
>
> Alas truth_valued_p is not quite what we want (and tweaking its
> definition has undesirable side-effects), so instead this patch
> introduces a new zero_one_valued predicate based on tree_nonzero_bits
> that extends truth_valued_p (which is for Boolean types with single
> bit precision). This is then used to simple if X*Y into X when
> both X and Y are zero_one_valued_p, and simplify X*Y into (-X) when
> X is zero_one_valued_p, in both cases replacing an integer multiplication
> with a cheaper bit-wise AND.
>
> This patch has been tested on x86_64-pc-linux-gnu with make bootstrap
> and make -k check, both with and with --target_board=unix{-m32}, with
> no new failures, except for a tweak required to tree-ssa/vrp116.c.
> The recently proposed cmove patch ensures the i386 backend continues
> to generate identical code for vrp116.c as before.
>
> Ok, either for mainline or when stage 1 reopens?
One issue is that on GIMPLE we consider (bit_and (negate @0) @1) to be
more costly than (mult @0 @1) because it is two operations rather than one.
So can we at least do (bit_and (negate! @0) @1) and thus require the negate
to be simplified?
Also the
+ && (TREE_CODE (@1) != INTEGER_CST
+ || wi::popcount (wi::to_wide (@1)) > 1))
exception is odd without providing the desired canonicalization to a shift?
In the end all this looks more like something for RTL (expansion?) where we
can query costs rather than canonicalization (to simpler expressions) which
is what we should do on GIMPLE.
Richard.
>
>
> 2022-04-20 Roger Sayle
> Andrew Pinski
> Jakub Jelinek
>
> gcc/ChangeLog
> PR middle-end/98865
> * match.pd (match zero_one_valued_p): New predicate.
> (mult @0 @1): Use zero_one_valued_p for transforming into (and @0
> @1).
> (mult zero_one_valued_p@0 @1): Convert integer multiplication into
> a negation and a bit-wise AND, if it can't be cheaply implemented by
> a single left shift.
>
> gcc/testsuite/ChangeLog
> PR middle-end/98865
> * gcc.dg/pr98865.c: New test case.
> * gcc.dg/vrp116.c: Tweak test to confirm the integer multiplication
> has been eliminated, not for the actual replacement implementation.
>
> Thanks,
> Roger
> --
>
[PATCH] tree-optimization/104240 - SLP discovery with swapped comparisons
The following extends SLP discovery to handle swapped operands
in comparisons.
Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.
2022-05-02 Richard Biener
PR tree-optimization/104240
* tree-vect-slp.cc (op1_op0_map): New.
(vect_get_operand_map): Handle compares.
(vect_build_slp_tree_1): Support swapped operands for
tcc_comparison.
* gcc.dg/vect/bb-slp-pr104240.c: New testcase.
---
gcc/testsuite/gcc.dg/vect/bb-slp-pr104240.c | 14 ++
gcc/tree-vect-slp.cc| 19 ++-
2 files changed, 32 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.dg/vect/bb-slp-pr104240.c
diff --git a/gcc/testsuite/gcc.dg/vect/bb-slp-pr104240.c
b/gcc/testsuite/gcc.dg/vect/bb-slp-pr104240.c
new file mode 100644
index 000..78905a468e0
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vect/bb-slp-pr104240.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target vect_float } */
+/* { dg-require-effective-target vect_int } */
+/* { dg-require-effective-target vect_cond_mixed } */
+
+void foo (int *c, float *x, float *y)
+{
+ c[0] = x[0] < y[0];
+ c[1] = y[1] > x[1];
+ c[2] = x[2] < y[2];
+ c[3] = x[3] < y[3];
+}
+
+/* { dg-final { scan-tree-dump "optimized: basic block" "slp2" } } */
diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc
index 0d400c00df1..2685bc10347 100644
--- a/gcc/tree-vect-slp.cc
+++ b/gcc/tree-vect-slp.cc
@@ -462,6 +462,7 @@ static const int cond_expr_maps[3][5] = {
static const int arg1_map[] = { 1, 1 };
static const int arg2_map[] = { 1, 2 };
static const int arg1_arg4_map[] = { 2, 1, 4 };
+static const int op1_op0_map[] = { 2, 1, 0 };
/* For most SLP statements, there is a one-to-one mapping between
gimple arguments and child nodes. If that is not true for STMT,
@@ -482,6 +483,9 @@ vect_get_operand_map (const gimple *stmt, unsigned char
swap = 0)
if (gimple_assign_rhs_code (assign) == COND_EXPR
&& COMPARISON_CLASS_P (gimple_assign_rhs1 (assign)))
return cond_expr_maps[swap];
+ if (TREE_CODE_CLASS (gimple_assign_rhs_code (assign)) == tcc_comparison
+ && swap)
+ return op1_op0_map;
}
gcc_assert (!swap);
if (auto call = dyn_cast (stmt))
@@ -1116,6 +1120,12 @@ vect_build_slp_tree_1 (vec_info *vinfo, unsigned char
*swap,
&& (alt_stmt_code == PLUS_EXPR
|| alt_stmt_code == MINUS_EXPR)
&& rhs_code == alt_stmt_code)
+ && !(first_stmt_code.is_tree_code ()
+ && rhs_code.is_tree_code ()
+ && (TREE_CODE_CLASS (tree_code (first_stmt_code))
+ == tcc_comparison)
+ && (swap_tree_comparison (tree_code (first_stmt_code))
+ == tree_code (rhs_code)))
&& !(STMT_VINFO_GROUPED_ACCESS (stmt_info)
&& (first_stmt_code == ARRAY_REF
|| first_stmt_code == BIT_FIELD_REF
@@ -1313,6 +1323,12 @@ vect_build_slp_tree_1 (vec_info *vinfo, unsigned char
*swap,
continue;
}
}
+
+ if (rhs_code.is_tree_code ()
+ && TREE_CODE_CLASS ((tree_code)rhs_code) == tcc_comparison
+ && (swap_tree_comparison ((tree_code)first_stmt_code)
+ == (tree_code)rhs_code))
+ swap[i] = 1;
}
matches[i] = true;
@@ -1326,7 +1342,8 @@ vect_build_slp_tree_1 (vec_info *vinfo, unsigned char
*swap,
with the permute we are going to use. */
if (alt_stmt_code != ERROR_MARK
&& (!alt_stmt_code.is_tree_code ()
- || TREE_CODE_CLASS (tree_code (alt_stmt_code)) != tcc_reference))
+ || (TREE_CODE_CLASS (tree_code (alt_stmt_code)) != tcc_reference
+ && TREE_CODE_CLASS (tree_code (alt_stmt_code)) !=
tcc_comparison)))
{
*two_operators = true;
}
--
2.34.1
[PATCH] c++: Don't emit deprecated warnings or unavailable errors on lambda declarations
Hi!
On the following testcase, we emit deprecated warnings or unavailable errors
even on merge declarations of those lambdas (the dg-bogus directives), while
IMHO we should emit them only when something actually calls those lambdas.
The following patch temporarily disables that diagnostics during
maybe_add_lambda_conv_op.
Ok for trunk if it passes bootstrap/regtest?
PR2173R1 also says that ambiguity between attribute-specifier-seq at the
end of requires-clause and attribute-specifier-seq from lambda-expression
should be resolved to attribute-specifier-seq for the latter. Do we need
to do anything about that? I mean, can a valid requires-clause end with
an attribute-specifier-seq? Say operator int [[]] is valid primary
expression, but requires operator int [[]] isn't valid, nor is
requires operator int, no?
2022-05-02 Jakub Jelinek
* lambda.cc: Include decl.h.
(maybe_add_lambda_conv_op): Temporarily override deprecated_state to
UNAVAILABLE_DEPRECATED_SUPPRESS.
* g++.dg/cpp23/lambda-attr1.C: New test.
* g++.dg/cpp23/lambda-attr2.C: New test.
--- gcc/cp/lambda.cc.jj 2022-05-02 07:10:02.360150857 +0200
+++ gcc/cp/lambda.cc2022-05-02 12:38:24.906950325 +0200
@@ -31,6 +31,7 @@ along with GCC; see the file COPYING3.
#include "toplev.h"
#include "gimplify.h"
#include "target.h"
+#include "decl.h"
/* Constructor for a lambda expression. */
@@ -1192,9 +1193,14 @@ maybe_add_lambda_conv_op (tree type)
}
}
else
-call = build_call_a (callop,
-direct_argvec->length (),
-direct_argvec->address ());
+{
+ /* Don't warn on deprecated or unavailable lambda declarations, unless
+the lambda is actually called. */
+ auto du = make_temp_override (deprecated_state,
+ UNAVAILABLE_DEPRECATED_SUPPRESS);
+ call = build_call_a (callop, direct_argvec->length (),
+ direct_argvec->address ());
+}
CALL_FROM_THUNK_P (call) = 1;
SET_EXPR_LOCATION (call, UNKNOWN_LOCATION);
--- gcc/testsuite/g++.dg/cpp23/lambda-attr1.C.jj2022-05-02
12:25:32.604570154 +0200
+++ gcc/testsuite/g++.dg/cpp23/lambda-attr1.C 2022-05-02 12:44:21.509041161
+0200
@@ -0,0 +1,37 @@
+// P2173R1 - Attributes on Lambda-Expressions
+// { dg-do compile { target c++11 } }
+
+void
+foo (bool x, bool y)
+{
+ auto a = [][[noreturn]] () {}; // { dg-warning "'noreturn' function
does return" }
+ if (x)
+a ();
+ auto b = [][[noreturn]] {}; // { dg-warning "'noreturn' function
does return" }
+ if (y)
+b ();
+ auto c = [] [[ deprecated ]] () {}; // { dg-bogus "is deprecated" }
+ c ();// { dg-warning "'foo\\\(bool,
bool\\\)::' is deprecated" }
+ auto d = [][[deprecated]] {};// { dg-bogus "is deprecated" }
+ d ();// { dg-warning "'foo\\\(bool,
bool\\\)::' is deprecated" }
+#if __cpp_generic_lambdas >= 201304
+ auto e = [] [[deprecated]] (auto x) {}; // { dg-bogus "is deprecated" }
+ e (0.0); // { dg-warning "'foo\\\(bool,
bool\\\)::\[^\n\r]*' is deprecated" "" { target c++14 } }
+#endif
+#if __cpp_generic_lambdas >= 201707
+ auto f = [] [[deprecated]] (T) {}; // { dg-bogus "is
deprecated" }
+ f (1); // { dg-warning "'foo\\\(bool,
bool\\\)::\[^\n\r]*' is deprecated" "" { target c++20 } }
+#endif
+ auto g = [][[nodiscard]](int) { return 1; };
+ g (1); // { dg-warning "ignoring return value
of 'foo\\\(bool, bool\\\)::', declared with attribute
'nodiscard'" }
+ auto h = [] [[nodiscard]] { return 0; };
+ h ();// { dg-warning "ignoring
return value of 'foo\\\(bool, bool\\\)::', declared with
attribute 'nodiscard'" }
+ auto i = [] [[ gnu::unavailable ]] () {};
+ auto j = [][[gnu::unavailable]] {};
+#if __cpp_generic_lambdas >= 201304
+ auto k = [] [[gnu::unavailable]] (auto x) {};// { dg-bogus "is
unavailable" }
+#endif
+#if __cpp_generic_lambdas >= 201707
+ auto l = [] [[gnu::unavailable]] (T) {};// { dg-bogus
"is unavailable" }
+#endif
+}
--- gcc/testsuite/g++.dg/cpp23/lambda-attr2.C.jj2022-05-02
12:44:30.099922893 +0200
+++ gcc/testsuite/g++.dg/cpp23/lambda-attr2.C 2022-05-02 12:48:29.234630831
+0200
@@ -0,0 +1,19 @@
+// P2173R1 - Attributes on Lambda-Expressions
+// { dg-do compile { target c++11 } }
+
+void
+foo (bool x, bool y)
+{
+ auto i = [] [[ gnu::unavailable ]] () {};
+ i ();// { dg-error "'foo\\\(bool,
bool\\\)::' is unavailable" }
+ auto j = [][[gnu::unavailable]] {};
+ j ();// { dg-error "'foo\\\(bool,
bool\\\)::' is unavailable" }
+#if __cpp_generic_lambdas >= 201304
+ auto k = [] [[gnu::unavailable]] (auto x) {};// { dg-bogus "is
unavailable" }
+ k (0.0);
Re: [PATCH] LTO plugin: add ld_plugin_version callback.
On 5/2/22 11:36, Richard Biener wrote: > Linkers will need heuristics anyway for older plugins and I think > LLVM could just behave here. Leaving that for the future, I may return to it. Right now, the benefit is pretty low to me. Martin
Re: [PATCH] LTO plugin: add ld_plugin_version callback.
On Mon, May 2, 2022 at 11:36 AM Richard Biener
wrote:
>
> On Mon, May 2, 2022 at 11:27 AM Martin Liška wrote:
> >
> > On 5/2/22 11:14, Richard Biener wrote:
> > > On Mon, May 2, 2022 at 11:08 AM Martin Liška wrote:
> > >>
> > >> On 5/2/22 11:03, Richard Biener wrote:
> > >>> In fact I think GCC is correct in its handling and LLVM would need to
> > >>> dup () the
> > >>> file descriptor. Was this issue raised with the LLVM folks?
> > >>
> > >> I don't know here.
> > >>
> > >>> I suspect the BFD
> > >>> linker will also not work with LLVM due to this?
> > >>
> > >> I'm not sure BFD is supported by LLVM:
> > >> https://llvm.org/docs/GoldPlugin.html
> > >>
> > >> They explicitly name it Gold plugin.
> > >
> > > BFD does
> > >
> > > if (plugin_call_claim_file (, ))
> > > einfo (_("%F%P: %s: plugin reported error claiming file\n"),
> > >plugin_error_plugin ());
> > >
> > > if (input->fd != -1
> > > && (!claimed || !bfd_plugin_target_p (ibfd->xvec)))
> > > {
> > > /* FIXME: fd belongs to us, not the plugin. GCC plugin, which
> > > doesn't need fd after plugin_call_claim_file, doesn't use
> > > BFD plugin target vector. Since GCC plugin doesn't call
> > > release_input_file, we close it here. LLVM plugin, which
> > > needs fd after plugin_call_claim_file and calls
> > > release_input_file after it is done, uses BFD plugin target
> > > vector. This scheme doesn't work when a plugin needs fd and
> > > doesn't use BFD plugin target vector neither. */
> > > release_plugin_file_descriptor (input);
> > > }
> > >
> > > so it knows about the problem. The above suggests that the plugin
> > > could set ->fd to -1 ...
> >
> > That would work. So we can add LDPT_REGISTER_CLAIM_FILE_HOOK_V2
> > hook that will release fd based on if claim_file_v2 resets it to -1 or not.
> >
> > Does it worth implementing?
>
> I'd say the folks to ask are the binutils, mold and lld linker plus of course
> LLVM which would need to adjust its handling. It would be also possible
> to add a int *claimed_fd argument or just explicitely document that
> the file descriptor belongs to the linker and thus just LLVM needs to be
> fixed to dup() it when it stores it away.
>
> Linkers will need heuristics anyway for older plugins and I think
> LLVM could just behave here.
Btw, binutils f4b78d1898203363e7f551497b6231d0f891d6f9
more accurately outlines the issue (though I admit I don't
fully understand the difference) and it hints at the linkers more
accurately need to track where the FD was obtained through.
Richard.
> Richard.
>
> >
> > Cheers,
> > Martin
> >
> > >
> > > Richard.
> > >
> > >>
> > >> Martin
> > >>
> >
Re: [PATCH] LTO plugin: add ld_plugin_version callback.
On Mon, May 2, 2022 at 11:27 AM Martin Liška wrote:
>
> On 5/2/22 11:14, Richard Biener wrote:
> > On Mon, May 2, 2022 at 11:08 AM Martin Liška wrote:
> >>
> >> On 5/2/22 11:03, Richard Biener wrote:
> >>> In fact I think GCC is correct in its handling and LLVM would need to dup
> >>> () the
> >>> file descriptor. Was this issue raised with the LLVM folks?
> >>
> >> I don't know here.
> >>
> >>> I suspect the BFD
> >>> linker will also not work with LLVM due to this?
> >>
> >> I'm not sure BFD is supported by LLVM:
> >> https://llvm.org/docs/GoldPlugin.html
> >>
> >> They explicitly name it Gold plugin.
> >
> > BFD does
> >
> > if (plugin_call_claim_file (, ))
> > einfo (_("%F%P: %s: plugin reported error claiming file\n"),
> >plugin_error_plugin ());
> >
> > if (input->fd != -1
> > && (!claimed || !bfd_plugin_target_p (ibfd->xvec)))
> > {
> > /* FIXME: fd belongs to us, not the plugin. GCC plugin, which
> > doesn't need fd after plugin_call_claim_file, doesn't use
> > BFD plugin target vector. Since GCC plugin doesn't call
> > release_input_file, we close it here. LLVM plugin, which
> > needs fd after plugin_call_claim_file and calls
> > release_input_file after it is done, uses BFD plugin target
> > vector. This scheme doesn't work when a plugin needs fd and
> > doesn't use BFD plugin target vector neither. */
> > release_plugin_file_descriptor (input);
> > }
> >
> > so it knows about the problem. The above suggests that the plugin
> > could set ->fd to -1 ...
>
> That would work. So we can add LDPT_REGISTER_CLAIM_FILE_HOOK_V2
> hook that will release fd based on if claim_file_v2 resets it to -1 or not.
>
> Does it worth implementing?
I'd say the folks to ask are the binutils, mold and lld linker plus of course
LLVM which would need to adjust its handling. It would be also possible
to add a int *claimed_fd argument or just explicitely document that
the file descriptor belongs to the linker and thus just LLVM needs to be
fixed to dup() it when it stores it away.
Linkers will need heuristics anyway for older plugins and I think
LLVM could just behave here.
Richard.
>
> Cheers,
> Martin
>
> >
> > Richard.
> >
> >>
> >> Martin
> >>
>
Re: [PATCH] LTO plugin: add ld_plugin_version callback.
On 5/2/22 11:14, Richard Biener wrote:
> On Mon, May 2, 2022 at 11:08 AM Martin Liška wrote:
>>
>> On 5/2/22 11:03, Richard Biener wrote:
>>> In fact I think GCC is correct in its handling and LLVM would need to dup
>>> () the
>>> file descriptor. Was this issue raised with the LLVM folks?
>>
>> I don't know here.
>>
>>> I suspect the BFD
>>> linker will also not work with LLVM due to this?
>>
>> I'm not sure BFD is supported by LLVM:
>> https://llvm.org/docs/GoldPlugin.html
>>
>> They explicitly name it Gold plugin.
>
> BFD does
>
> if (plugin_call_claim_file (, ))
> einfo (_("%F%P: %s: plugin reported error claiming file\n"),
>plugin_error_plugin ());
>
> if (input->fd != -1
> && (!claimed || !bfd_plugin_target_p (ibfd->xvec)))
> {
> /* FIXME: fd belongs to us, not the plugin. GCC plugin, which
> doesn't need fd after plugin_call_claim_file, doesn't use
> BFD plugin target vector. Since GCC plugin doesn't call
> release_input_file, we close it here. LLVM plugin, which
> needs fd after plugin_call_claim_file and calls
> release_input_file after it is done, uses BFD plugin target
> vector. This scheme doesn't work when a plugin needs fd and
> doesn't use BFD plugin target vector neither. */
> release_plugin_file_descriptor (input);
> }
>
> so it knows about the problem. The above suggests that the plugin
> could set ->fd to -1 ...
That would work. So we can add LDPT_REGISTER_CLAIM_FILE_HOOK_V2
hook that will release fd based on if claim_file_v2 resets it to -1 or not.
Does it worth implementing?
Cheers,
Martin
>
> Richard.
>
>>
>> Martin
>>
Re: [PATCH] LTO plugin: add ld_plugin_version callback.
On Mon, May 2, 2022 at 11:08 AM Martin Liška wrote:
>
> On 5/2/22 11:03, Richard Biener wrote:
> > In fact I think GCC is correct in its handling and LLVM would need to dup
> > () the
> > file descriptor. Was this issue raised with the LLVM folks?
>
> I don't know here.
>
> > I suspect the BFD
> > linker will also not work with LLVM due to this?
>
> I'm not sure BFD is supported by LLVM:
> https://llvm.org/docs/GoldPlugin.html
>
> They explicitly name it Gold plugin.
BFD does
if (plugin_call_claim_file (, ))
einfo (_("%F%P: %s: plugin reported error claiming file\n"),
plugin_error_plugin ());
if (input->fd != -1
&& (!claimed || !bfd_plugin_target_p (ibfd->xvec)))
{
/* FIXME: fd belongs to us, not the plugin. GCC plugin, which
doesn't need fd after plugin_call_claim_file, doesn't use
BFD plugin target vector. Since GCC plugin doesn't call
release_input_file, we close it here. LLVM plugin, which
needs fd after plugin_call_claim_file and calls
release_input_file after it is done, uses BFD plugin target
vector. This scheme doesn't work when a plugin needs fd and
doesn't use BFD plugin target vector neither. */
release_plugin_file_descriptor (input);
}
so it knows about the problem. The above suggests that the plugin
could set ->fd to -1 ...
Richard.
>
> Martin
>
Re: [PATCH] LTO plugin: add ld_plugin_version callback.
On Mon, May 2, 2022 at 11:06 AM Jan Hubicka wrote: > > > On Mon, May 2, 2022 at 10:51 AM Richard Biener > > wrote: > > > > > > On Mon, May 2, 2022 at 10:19 AM Martin Liška wrote: > > > > > > > > On 5/2/22 10:09, Richard Biener wrote: > > > > > On Mon, May 2, 2022 at 9:52 AM Martin Liška wrote: > > > > >> > > > > >> Hi. > > > > >> > > > > >> This in a new plug-in function that helps identifying compiler > > > > >> by a linker. Will be used in mold linker. > > > > >> > > > > >> Patch can bootstrap on x86_64-linux-gnu and survives regression > > > > >> tests. > > > > >> > > > > >> Ready to be installed? > > > > > > > > > > It looks a bit backward to query the compiler (and even more so to > > > > > have a fixed set) from the linker. What is this going to be used for? > > > > > > > > It's going to be used by mold, there are small behavior divergence > > > > in between LLVM and GCC, where one compiler closes provided file > > > > descriptors: > > > > https://github.com/rui314/mold/blob/a15f3875269b575d024725590ae3ef87b611b8d8/elf/lto.cc#L545-L550 > > > > > > But that then seems to be a defect in the plugin API specification (or > > > one of the two implementations). Instead of adding a new API entry > > > that defect should be fixed - both require changes in the actual plugin > > > specification. > > > > implementation of course. > > I agree that implementation should be fixed. The version query however > makes sense to me. It makes it possible for linker to work around bugs of > older compilers while implementing features that in general rely on > corrected behaviour. > > I however also find bit odd to have fixed list gcc and llvm. The plugin > API (when implemented without bugs) should be compler independent. In > this case it would make more sense to have string for compiler name. OK, though this doesn't help for bugs in the tool chain that does not yet implement this version hook so the very reason for the hook is yet unknown bugs. It's then also tempting to never fix the actual bugs but rely on an ever growing list of version/bug workarounds in linkers :/ > I think Martin was following existing API for querying linker type > which has same oddity and uses enum to represent known linkers > supporting the plugin API? > > Honza
Re: [PATCH] LTO plugin: add ld_plugin_version callback.
On 5/2/22 11:03, Richard Biener wrote: > In fact I think GCC is correct in its handling and LLVM would need to dup () > the > file descriptor. Was this issue raised with the LLVM folks? I don't know here. > I suspect the BFD > linker will also not work with LLVM due to this? I'm not sure BFD is supported by LLVM: https://llvm.org/docs/GoldPlugin.html They explicitly name it Gold plugin. Martin
Re: [PATCH] LTO plugin: add ld_plugin_version callback.
> On Mon, May 2, 2022 at 10:51 AM Richard Biener > wrote: > > > > On Mon, May 2, 2022 at 10:19 AM Martin Liška wrote: > > > > > > On 5/2/22 10:09, Richard Biener wrote: > > > > On Mon, May 2, 2022 at 9:52 AM Martin Liška wrote: > > > >> > > > >> Hi. > > > >> > > > >> This in a new plug-in function that helps identifying compiler > > > >> by a linker. Will be used in mold linker. > > > >> > > > >> Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > > > >> > > > >> Ready to be installed? > > > > > > > > It looks a bit backward to query the compiler (and even more so to > > > > have a fixed set) from the linker. What is this going to be used for? > > > > > > It's going to be used by mold, there are small behavior divergence > > > in between LLVM and GCC, where one compiler closes provided file > > > descriptors: > > > https://github.com/rui314/mold/blob/a15f3875269b575d024725590ae3ef87b611b8d8/elf/lto.cc#L545-L550 > > > > But that then seems to be a defect in the plugin API specification (or > > one of the two implementations). Instead of adding a new API entry > > that defect should be fixed - both require changes in the actual plugin > > specification. > > implementation of course. I agree that implementation should be fixed. The version query however makes sense to me. It makes it possible for linker to work around bugs of older compilers while implementing features that in general rely on corrected behaviour. I however also find bit odd to have fixed list gcc and llvm. The plugin API (when implemented without bugs) should be compler independent. In this case it would make more sense to have string for compiler name. I think Martin was following existing API for querying linker type which has same oddity and uses enum to represent known linkers supporting the plugin API? Honza
Re: [PATCH] LTO plugin: add ld_plugin_version callback.
On 5/2/22 11:00, Richard Biener wrote:
> On Mon, May 2, 2022 at 10:57 AM Martin Liška wrote:
>>
>> On 5/2/22 10:51, Richard Biener wrote:
>>> On Mon, May 2, 2022 at 10:19 AM Martin Liška wrote:
On 5/2/22 10:09, Richard Biener wrote:
> On Mon, May 2, 2022 at 9:52 AM Martin Liška wrote:
>>
>> Hi.
>>
>> This in a new plug-in function that helps identifying compiler
>> by a linker. Will be used in mold linker.
>>
>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>
>> Ready to be installed?
>
> It looks a bit backward to query the compiler (and even more so to
> have a fixed set) from the linker. What is this going to be used for?
It's going to be used by mold, there are small behavior divergence
in between LLVM and GCC, where one compiler closes provided file
descriptors:
https://github.com/rui314/mold/blob/a15f3875269b575d024725590ae3ef87b611b8d8/elf/lto.cc#L545-L550
>>>
>>> But that then seems to be a defect in the plugin API specification (or
>>> one of the two implementations). Instead of adding a new API entry
>>> that defect should be fixed - both require changes in the actual plugin
>>> specification.
>>
>> Well, the question is if it was specified somewhere or not. Plus there are
>> released
>> versions of both compilers and plug-ins, so it's not easily fixable :/
>
> Well, without changing plug-ins the new version callback doesn't exist
> either. The
> problem of existing discrepancies is not fixable without heuristics in
> the linker,
> the _actual_ problem can of course be fixed but the solution is of course
> _not_
> to make if (gcc) or if (llvm) but if then something like if (plugin
> closed fd) or
Yes, feature querying would be much better approach.
> specify the plugin has to or has not to close fds passed in
> ld_plugin_input_file.
Anyway, I don't want to over-engineer things here and I think the mold
linker can live with some heuristics.
That said, I don't insist on this patch.
Martin
>
> Richard.
>
>> Martin
>>
>>>
>>> Richard.
>>>
> If then this should probably receive a unformatted string the linker
> has to decipher itself like "gcc (SUSE Linux) 7.5.0" or something
> (what we produce with gcc --version), and clang would produce
> "clang version 11.0.1" or so?
Well, it brings its own set of problems such string parsing. Note that the
plugin API
currently provides something like:
/* The version of gold being used, or -1 if not gold. The number is
MAJOR * 100 + MINOR. */
static int gold_version = -1;
So my suggestion is quite aligned with that.
Martin
>
> Richard.
>
>> Thanks,
>> Martin
>>
>> include/ChangeLog:
>>
>> * plugin-api.h (enum ld_plugin_compiler): New enum.
>> (enum ld_plugin_version): New typedef.
>> (struct ld_plugin_tv): Add new field.
>>
>> lto-plugin/ChangeLog:
>>
>> * lto-plugin.c (onload): Call ld_plugin_version.
>> ---
>> include/plugin-api.h| 18 +-
>> lto-plugin/lto-plugin.c | 4
>> 2 files changed, 21 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/plugin-api.h b/include/plugin-api.h
>> index 4e12c0320d6..4d0989028cc 100644
>> --- a/include/plugin-api.h
>> +++ b/include/plugin-api.h
>> @@ -211,6 +211,13 @@ enum ld_plugin_symbol_section_kind
>>LDSSK_BSS
>> };
>>
>> +enum ld_plugin_compiler
>> +{
>> + LDPC_UNKNOWN,
>> + LDPC_GCC,
>> + LDPC_LLVM
>> +};
>> +
>> /* How a symbol is resolved. */
>>
>> enum ld_plugin_symbol_resolution
>> @@ -475,6 +482,13 @@ enum ld_plugin_status
>> (*ld_plugin_get_wrap_symbols) (uint64_t *num_symbols,
>> const char ***wrap_symbol_list);
>>
>> +/* The linker's interface for registering the "plugin_version" handler.
>> + This handler is called directly after onload and provides compiler
>> + and its number version (MAJOR * 1000 + MINOR). */
>> +
>> +typedef
>> +void (*ld_plugin_version) (enum ld_plugin_compiler compiler, int
>> version);
>> +
>> enum ld_plugin_level
>> {
>>LDPL_INFO,
>> @@ -520,7 +534,8 @@ enum ld_plugin_tag
>>LDPT_GET_INPUT_SECTION_SIZE = 30,
>>LDPT_REGISTER_NEW_INPUT_HOOK = 31,
>>LDPT_GET_WRAP_SYMBOLS = 32,
>> - LDPT_ADD_SYMBOLS_V2 = 33
>> + LDPT_ADD_SYMBOLS_V2 = 33,
>> + LDPT_PLUGIN_VERSION = 34,
>> };
>>
>> /* The plugin transfer vector. */
>> @@ -556,6 +571,7 @@ struct ld_plugin_tv
>> ld_plugin_get_input_section_size tv_get_input_section_size;
>> ld_plugin_register_new_input tv_register_new_input;
>> ld_plugin_get_wrap_symbols tv_get_wrap_symbols;
>> +ld_plugin_version tv_plugin_version;
Re: [PATCH] LTO plugin: add ld_plugin_version callback.
On Mon, May 2, 2022 at 11:00 AM Richard Biener
wrote:
>
> On Mon, May 2, 2022 at 10:57 AM Martin Liška wrote:
> >
> > On 5/2/22 10:51, Richard Biener wrote:
> > > On Mon, May 2, 2022 at 10:19 AM Martin Liška wrote:
> > >>
> > >> On 5/2/22 10:09, Richard Biener wrote:
> > >>> On Mon, May 2, 2022 at 9:52 AM Martin Liška wrote:
> >
> > Hi.
> >
> > This in a new plug-in function that helps identifying compiler
> > by a linker. Will be used in mold linker.
> >
> > Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> >
> > Ready to be installed?
> > >>>
> > >>> It looks a bit backward to query the compiler (and even more so to
> > >>> have a fixed set) from the linker. What is this going to be used for?
> > >>
> > >> It's going to be used by mold, there are small behavior divergence
> > >> in between LLVM and GCC, where one compiler closes provided file
> > >> descriptors:
> > >> https://github.com/rui314/mold/blob/a15f3875269b575d024725590ae3ef87b611b8d8/elf/lto.cc#L545-L550
> > >
> > > But that then seems to be a defect in the plugin API specification (or
> > > one of the two implementations). Instead of adding a new API entry
> > > that defect should be fixed - both require changes in the actual plugin
> > > specification.
> >
> > Well, the question is if it was specified somewhere or not. Plus there are
> > released
> > versions of both compilers and plug-ins, so it's not easily fixable :/
>
> Well, without changing plug-ins the new version callback doesn't exist
> either. The
> problem of existing discrepancies is not fixable without heuristics in
> the linker,
> the _actual_ problem can of course be fixed but the solution is of course
> _not_
> to make if (gcc) or if (llvm) but if then something like if (plugin
> closed fd) or
> specify the plugin has to or has not to close fds passed in
> ld_plugin_input_file.
In fact I think GCC is correct in its handling and LLVM would need to dup () the
file descriptor. Was this issue raised with the LLVM folks? I suspect the BFD
linker will also not work with LLVM due to this?
Richard.
> Richard.
>
> > Martin
> >
> > >
> > > Richard.
> > >
> > >>> If then this should probably receive a unformatted string the linker
> > >>> has to decipher itself like "gcc (SUSE Linux) 7.5.0" or something
> > >>> (what we produce with gcc --version), and clang would produce
> > >>> "clang version 11.0.1" or so?
> > >>
> > >> Well, it brings its own set of problems such string parsing. Note that
> > >> the plugin API
> > >> currently provides something like:
> > >>
> > >> /* The version of gold being used, or -1 if not gold. The number is
> > >>MAJOR * 100 + MINOR. */
> > >> static int gold_version = -1;
> > >>
> > >> So my suggestion is quite aligned with that.
> > >>
> > >> Martin
> > >>
> > >>>
> > >>> Richard.
> > >>>
> > Thanks,
> > Martin
> >
> > include/ChangeLog:
> >
> > * plugin-api.h (enum ld_plugin_compiler): New enum.
> > (enum ld_plugin_version): New typedef.
> > (struct ld_plugin_tv): Add new field.
> >
> > lto-plugin/ChangeLog:
> >
> > * lto-plugin.c (onload): Call ld_plugin_version.
> > ---
> > include/plugin-api.h| 18 +-
> > lto-plugin/lto-plugin.c | 4
> > 2 files changed, 21 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/plugin-api.h b/include/plugin-api.h
> > index 4e12c0320d6..4d0989028cc 100644
> > --- a/include/plugin-api.h
> > +++ b/include/plugin-api.h
> > @@ -211,6 +211,13 @@ enum ld_plugin_symbol_section_kind
> > LDSSK_BSS
> > };
> >
> > +enum ld_plugin_compiler
> > +{
> > + LDPC_UNKNOWN,
> > + LDPC_GCC,
> > + LDPC_LLVM
> > +};
> > +
> > /* How a symbol is resolved. */
> >
> > enum ld_plugin_symbol_resolution
> > @@ -475,6 +482,13 @@ enum ld_plugin_status
> > (*ld_plugin_get_wrap_symbols) (uint64_t *num_symbols,
> > const char ***wrap_symbol_list);
> >
> > +/* The linker's interface for registering the "plugin_version"
> > handler.
> > + This handler is called directly after onload and provides compiler
> > + and its number version (MAJOR * 1000 + MINOR). */
> > +
> > +typedef
> > +void (*ld_plugin_version) (enum ld_plugin_compiler compiler, int
> > version);
> > +
> > enum ld_plugin_level
> > {
> > LDPL_INFO,
> > @@ -520,7 +534,8 @@ enum ld_plugin_tag
> > LDPT_GET_INPUT_SECTION_SIZE = 30,
> > LDPT_REGISTER_NEW_INPUT_HOOK = 31,
> > LDPT_GET_WRAP_SYMBOLS = 32,
> > - LDPT_ADD_SYMBOLS_V2 = 33
> > + LDPT_ADD_SYMBOLS_V2 = 33,
> > + LDPT_PLUGIN_VERSION = 34,
> > };
> >
> > /* The plugin transfer vector. */
> > @@ -556,6 +571,7 @@ struct
Re: [PATCH] LTO plugin: add ld_plugin_version callback.
On Mon, May 2, 2022 at 10:57 AM Martin Liška wrote:
>
> On 5/2/22 10:51, Richard Biener wrote:
> > On Mon, May 2, 2022 at 10:19 AM Martin Liška wrote:
> >>
> >> On 5/2/22 10:09, Richard Biener wrote:
> >>> On Mon, May 2, 2022 at 9:52 AM Martin Liška wrote:
>
> Hi.
>
> This in a new plug-in function that helps identifying compiler
> by a linker. Will be used in mold linker.
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ready to be installed?
> >>>
> >>> It looks a bit backward to query the compiler (and even more so to
> >>> have a fixed set) from the linker. What is this going to be used for?
> >>
> >> It's going to be used by mold, there are small behavior divergence
> >> in between LLVM and GCC, where one compiler closes provided file
> >> descriptors:
> >> https://github.com/rui314/mold/blob/a15f3875269b575d024725590ae3ef87b611b8d8/elf/lto.cc#L545-L550
> >
> > But that then seems to be a defect in the plugin API specification (or
> > one of the two implementations). Instead of adding a new API entry
> > that defect should be fixed - both require changes in the actual plugin
> > specification.
>
> Well, the question is if it was specified somewhere or not. Plus there are
> released
> versions of both compilers and plug-ins, so it's not easily fixable :/
Well, without changing plug-ins the new version callback doesn't exist
either. The
problem of existing discrepancies is not fixable without heuristics in
the linker,
the _actual_ problem can of course be fixed but the solution is of course _not_
to make if (gcc) or if (llvm) but if then something like if (plugin
closed fd) or
specify the plugin has to or has not to close fds passed in
ld_plugin_input_file.
Richard.
> Martin
>
> >
> > Richard.
> >
> >>> If then this should probably receive a unformatted string the linker
> >>> has to decipher itself like "gcc (SUSE Linux) 7.5.0" or something
> >>> (what we produce with gcc --version), and clang would produce
> >>> "clang version 11.0.1" or so?
> >>
> >> Well, it brings its own set of problems such string parsing. Note that the
> >> plugin API
> >> currently provides something like:
> >>
> >> /* The version of gold being used, or -1 if not gold. The number is
> >>MAJOR * 100 + MINOR. */
> >> static int gold_version = -1;
> >>
> >> So my suggestion is quite aligned with that.
> >>
> >> Martin
> >>
> >>>
> >>> Richard.
> >>>
> Thanks,
> Martin
>
> include/ChangeLog:
>
> * plugin-api.h (enum ld_plugin_compiler): New enum.
> (enum ld_plugin_version): New typedef.
> (struct ld_plugin_tv): Add new field.
>
> lto-plugin/ChangeLog:
>
> * lto-plugin.c (onload): Call ld_plugin_version.
> ---
> include/plugin-api.h| 18 +-
> lto-plugin/lto-plugin.c | 4
> 2 files changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/include/plugin-api.h b/include/plugin-api.h
> index 4e12c0320d6..4d0989028cc 100644
> --- a/include/plugin-api.h
> +++ b/include/plugin-api.h
> @@ -211,6 +211,13 @@ enum ld_plugin_symbol_section_kind
> LDSSK_BSS
> };
>
> +enum ld_plugin_compiler
> +{
> + LDPC_UNKNOWN,
> + LDPC_GCC,
> + LDPC_LLVM
> +};
> +
> /* How a symbol is resolved. */
>
> enum ld_plugin_symbol_resolution
> @@ -475,6 +482,13 @@ enum ld_plugin_status
> (*ld_plugin_get_wrap_symbols) (uint64_t *num_symbols,
> const char ***wrap_symbol_list);
>
> +/* The linker's interface for registering the "plugin_version" handler.
> + This handler is called directly after onload and provides compiler
> + and its number version (MAJOR * 1000 + MINOR). */
> +
> +typedef
> +void (*ld_plugin_version) (enum ld_plugin_compiler compiler, int
> version);
> +
> enum ld_plugin_level
> {
> LDPL_INFO,
> @@ -520,7 +534,8 @@ enum ld_plugin_tag
> LDPT_GET_INPUT_SECTION_SIZE = 30,
> LDPT_REGISTER_NEW_INPUT_HOOK = 31,
> LDPT_GET_WRAP_SYMBOLS = 32,
> - LDPT_ADD_SYMBOLS_V2 = 33
> + LDPT_ADD_SYMBOLS_V2 = 33,
> + LDPT_PLUGIN_VERSION = 34,
> };
>
> /* The plugin transfer vector. */
> @@ -556,6 +571,7 @@ struct ld_plugin_tv
> ld_plugin_get_input_section_size tv_get_input_section_size;
> ld_plugin_register_new_input tv_register_new_input;
> ld_plugin_get_wrap_symbols tv_get_wrap_symbols;
> +ld_plugin_version tv_plugin_version;
> } tv_u;
> };
>
> diff --git a/lto-plugin/lto-plugin.c b/lto-plugin/lto-plugin.c
> index 7a1c77812f6..698a0617ca7 100644
> --- a/lto-plugin/lto-plugin.c
> +++ b/lto-plugin/lto-plugin.c
> @@ -69,6 +69,7 @@ along with this program; see the file
Re: [PATCH] LTO plugin: add ld_plugin_version callback.
On Mon, May 2, 2022 at 10:54 AM Richard Biener
wrote:
>
> On Mon, May 2, 2022 at 10:51 AM Richard Biener
> wrote:
> >
> > On Mon, May 2, 2022 at 10:19 AM Martin Liška wrote:
> > >
> > > On 5/2/22 10:09, Richard Biener wrote:
> > > > On Mon, May 2, 2022 at 9:52 AM Martin Liška wrote:
> > > >>
> > > >> Hi.
> > > >>
> > > >> This in a new plug-in function that helps identifying compiler
> > > >> by a linker. Will be used in mold linker.
> > > >>
> > > >> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> > > >>
> > > >> Ready to be installed?
> > > >
> > > > It looks a bit backward to query the compiler (and even more so to
> > > > have a fixed set) from the linker. What is this going to be used for?
> > >
> > > It's going to be used by mold, there are small behavior divergence
> > > in between LLVM and GCC, where one compiler closes provided file
> > > descriptors:
> > > https://github.com/rui314/mold/blob/a15f3875269b575d024725590ae3ef87b611b8d8/elf/lto.cc#L545-L550
> >
> > But that then seems to be a defect in the plugin API specification (or
> > one of the two implementations). Instead of adding a new API entry
> > that defect should be fixed - both require changes in the actual plugin
> > specification.
>
> implementation of course.
So for example a claim_file_v2 entry or alternatively amending
ld_plugin_input_file to denote whether the callee closed the fd
(set it to -1?) could be possible as well if we want to support both
keeping the file open or closing it. Btw, struct layout/meaning changes
are always more difficult of course.
Richard.
> Richard.
>
> >
> > Richard.
> >
> > > > If then this should probably receive a unformatted string the linker
> > > > has to decipher itself like "gcc (SUSE Linux) 7.5.0" or something
> > > > (what we produce with gcc --version), and clang would produce
> > > > "clang version 11.0.1" or so?
> > >
> > > Well, it brings its own set of problems such string parsing. Note that
> > > the plugin API
> > > currently provides something like:
> > >
> > > /* The version of gold being used, or -1 if not gold. The number is
> > >MAJOR * 100 + MINOR. */
> > > static int gold_version = -1;
> > >
> > > So my suggestion is quite aligned with that.
> > >
> > > Martin
> > >
> > > >
> > > > Richard.
> > > >
> > > >> Thanks,
> > > >> Martin
> > > >>
> > > >> include/ChangeLog:
> > > >>
> > > >> * plugin-api.h (enum ld_plugin_compiler): New enum.
> > > >> (enum ld_plugin_version): New typedef.
> > > >> (struct ld_plugin_tv): Add new field.
> > > >>
> > > >> lto-plugin/ChangeLog:
> > > >>
> > > >> * lto-plugin.c (onload): Call ld_plugin_version.
> > > >> ---
> > > >> include/plugin-api.h| 18 +-
> > > >> lto-plugin/lto-plugin.c | 4
> > > >> 2 files changed, 21 insertions(+), 1 deletion(-)
> > > >>
> > > >> diff --git a/include/plugin-api.h b/include/plugin-api.h
> > > >> index 4e12c0320d6..4d0989028cc 100644
> > > >> --- a/include/plugin-api.h
> > > >> +++ b/include/plugin-api.h
> > > >> @@ -211,6 +211,13 @@ enum ld_plugin_symbol_section_kind
> > > >>LDSSK_BSS
> > > >> };
> > > >>
> > > >> +enum ld_plugin_compiler
> > > >> +{
> > > >> + LDPC_UNKNOWN,
> > > >> + LDPC_GCC,
> > > >> + LDPC_LLVM
> > > >> +};
> > > >> +
> > > >> /* How a symbol is resolved. */
> > > >>
> > > >> enum ld_plugin_symbol_resolution
> > > >> @@ -475,6 +482,13 @@ enum ld_plugin_status
> > > >> (*ld_plugin_get_wrap_symbols) (uint64_t *num_symbols,
> > > >> const char ***wrap_symbol_list);
> > > >>
> > > >> +/* The linker's interface for registering the "plugin_version"
> > > >> handler.
> > > >> + This handler is called directly after onload and provides compiler
> > > >> + and its number version (MAJOR * 1000 + MINOR). */
> > > >> +
> > > >> +typedef
> > > >> +void (*ld_plugin_version) (enum ld_plugin_compiler compiler, int
> > > >> version);
> > > >> +
> > > >> enum ld_plugin_level
> > > >> {
> > > >>LDPL_INFO,
> > > >> @@ -520,7 +534,8 @@ enum ld_plugin_tag
> > > >>LDPT_GET_INPUT_SECTION_SIZE = 30,
> > > >>LDPT_REGISTER_NEW_INPUT_HOOK = 31,
> > > >>LDPT_GET_WRAP_SYMBOLS = 32,
> > > >> - LDPT_ADD_SYMBOLS_V2 = 33
> > > >> + LDPT_ADD_SYMBOLS_V2 = 33,
> > > >> + LDPT_PLUGIN_VERSION = 34,
> > > >> };
> > > >>
> > > >> /* The plugin transfer vector. */
> > > >> @@ -556,6 +571,7 @@ struct ld_plugin_tv
> > > >> ld_plugin_get_input_section_size tv_get_input_section_size;
> > > >> ld_plugin_register_new_input tv_register_new_input;
> > > >> ld_plugin_get_wrap_symbols tv_get_wrap_symbols;
> > > >> +ld_plugin_version tv_plugin_version;
> > > >>} tv_u;
> > > >> };
> > > >>
> > > >> diff --git a/lto-plugin/lto-plugin.c b/lto-plugin/lto-plugin.c
> > > >> index 7a1c77812f6..698a0617ca7 100644
> > > >> --- a/lto-plugin/lto-plugin.c
> > > >> +++ b/lto-plugin/lto-plugin.c
> > > >> @@ -69,6 +69,7 @@ along with this program; see the file
Re: [PATCH] LTO plugin: add ld_plugin_version callback.
On 5/2/22 10:51, Richard Biener wrote:
> On Mon, May 2, 2022 at 10:19 AM Martin Liška wrote:
>>
>> On 5/2/22 10:09, Richard Biener wrote:
>>> On Mon, May 2, 2022 at 9:52 AM Martin Liška wrote:
Hi.
This in a new plug-in function that helps identifying compiler
by a linker. Will be used in mold linker.
Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
Ready to be installed?
>>>
>>> It looks a bit backward to query the compiler (and even more so to
>>> have a fixed set) from the linker. What is this going to be used for?
>>
>> It's going to be used by mold, there are small behavior divergence
>> in between LLVM and GCC, where one compiler closes provided file descriptors:
>> https://github.com/rui314/mold/blob/a15f3875269b575d024725590ae3ef87b611b8d8/elf/lto.cc#L545-L550
>
> But that then seems to be a defect in the plugin API specification (or
> one of the two implementations). Instead of adding a new API entry
> that defect should be fixed - both require changes in the actual plugin
> specification.
Well, the question is if it was specified somewhere or not. Plus there are
released
versions of both compilers and plug-ins, so it's not easily fixable :/
Martin
>
> Richard.
>
>>> If then this should probably receive a unformatted string the linker
>>> has to decipher itself like "gcc (SUSE Linux) 7.5.0" or something
>>> (what we produce with gcc --version), and clang would produce
>>> "clang version 11.0.1" or so?
>>
>> Well, it brings its own set of problems such string parsing. Note that the
>> plugin API
>> currently provides something like:
>>
>> /* The version of gold being used, or -1 if not gold. The number is
>>MAJOR * 100 + MINOR. */
>> static int gold_version = -1;
>>
>> So my suggestion is quite aligned with that.
>>
>> Martin
>>
>>>
>>> Richard.
>>>
Thanks,
Martin
include/ChangeLog:
* plugin-api.h (enum ld_plugin_compiler): New enum.
(enum ld_plugin_version): New typedef.
(struct ld_plugin_tv): Add new field.
lto-plugin/ChangeLog:
* lto-plugin.c (onload): Call ld_plugin_version.
---
include/plugin-api.h| 18 +-
lto-plugin/lto-plugin.c | 4
2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/include/plugin-api.h b/include/plugin-api.h
index 4e12c0320d6..4d0989028cc 100644
--- a/include/plugin-api.h
+++ b/include/plugin-api.h
@@ -211,6 +211,13 @@ enum ld_plugin_symbol_section_kind
LDSSK_BSS
};
+enum ld_plugin_compiler
+{
+ LDPC_UNKNOWN,
+ LDPC_GCC,
+ LDPC_LLVM
+};
+
/* How a symbol is resolved. */
enum ld_plugin_symbol_resolution
@@ -475,6 +482,13 @@ enum ld_plugin_status
(*ld_plugin_get_wrap_symbols) (uint64_t *num_symbols,
const char ***wrap_symbol_list);
+/* The linker's interface for registering the "plugin_version" handler.
+ This handler is called directly after onload and provides compiler
+ and its number version (MAJOR * 1000 + MINOR). */
+
+typedef
+void (*ld_plugin_version) (enum ld_plugin_compiler compiler, int version);
+
enum ld_plugin_level
{
LDPL_INFO,
@@ -520,7 +534,8 @@ enum ld_plugin_tag
LDPT_GET_INPUT_SECTION_SIZE = 30,
LDPT_REGISTER_NEW_INPUT_HOOK = 31,
LDPT_GET_WRAP_SYMBOLS = 32,
- LDPT_ADD_SYMBOLS_V2 = 33
+ LDPT_ADD_SYMBOLS_V2 = 33,
+ LDPT_PLUGIN_VERSION = 34,
};
/* The plugin transfer vector. */
@@ -556,6 +571,7 @@ struct ld_plugin_tv
ld_plugin_get_input_section_size tv_get_input_section_size;
ld_plugin_register_new_input tv_register_new_input;
ld_plugin_get_wrap_symbols tv_get_wrap_symbols;
+ld_plugin_version tv_plugin_version;
} tv_u;
};
diff --git a/lto-plugin/lto-plugin.c b/lto-plugin/lto-plugin.c
index 7a1c77812f6..698a0617ca7 100644
--- a/lto-plugin/lto-plugin.c
+++ b/lto-plugin/lto-plugin.c
@@ -69,6 +69,7 @@ along with this program; see the file COPYING3. If not
see
#include "../gcc/lto/common.h"
#include "simple-object.h"
#include "plugin-api.h"
+#include "ansidecl.h"
/* We need to use I64 instead of ll width-specifier on native Windows.
The reason for this is that older MS-runtimes don't support the ll. */
@@ -1466,6 +1467,9 @@ onload (struct ld_plugin_tv *tv)
/* We only use this to make user-friendly temp file names. */
link_output_name = p->tv_u.tv_string;
break;
+ case LDPT_PLUGIN_VERSION:
+ p->tv_u.tv_plugin_version (LDPC_GCC, GCC_VERSION);
+ break;
default:
break;
}
--
2.36.0
>>
Re: [PATCH] LTO plugin: add ld_plugin_version callback.
On Mon, May 2, 2022 at 10:51 AM Richard Biener
wrote:
>
> On Mon, May 2, 2022 at 10:19 AM Martin Liška wrote:
> >
> > On 5/2/22 10:09, Richard Biener wrote:
> > > On Mon, May 2, 2022 at 9:52 AM Martin Liška wrote:
> > >>
> > >> Hi.
> > >>
> > >> This in a new plug-in function that helps identifying compiler
> > >> by a linker. Will be used in mold linker.
> > >>
> > >> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> > >>
> > >> Ready to be installed?
> > >
> > > It looks a bit backward to query the compiler (and even more so to
> > > have a fixed set) from the linker. What is this going to be used for?
> >
> > It's going to be used by mold, there are small behavior divergence
> > in between LLVM and GCC, where one compiler closes provided file
> > descriptors:
> > https://github.com/rui314/mold/blob/a15f3875269b575d024725590ae3ef87b611b8d8/elf/lto.cc#L545-L550
>
> But that then seems to be a defect in the plugin API specification (or
> one of the two implementations). Instead of adding a new API entry
> that defect should be fixed - both require changes in the actual plugin
> specification.
implementation of course.
Richard.
>
> Richard.
>
> > > If then this should probably receive a unformatted string the linker
> > > has to decipher itself like "gcc (SUSE Linux) 7.5.0" or something
> > > (what we produce with gcc --version), and clang would produce
> > > "clang version 11.0.1" or so?
> >
> > Well, it brings its own set of problems such string parsing. Note that the
> > plugin API
> > currently provides something like:
> >
> > /* The version of gold being used, or -1 if not gold. The number is
> >MAJOR * 100 + MINOR. */
> > static int gold_version = -1;
> >
> > So my suggestion is quite aligned with that.
> >
> > Martin
> >
> > >
> > > Richard.
> > >
> > >> Thanks,
> > >> Martin
> > >>
> > >> include/ChangeLog:
> > >>
> > >> * plugin-api.h (enum ld_plugin_compiler): New enum.
> > >> (enum ld_plugin_version): New typedef.
> > >> (struct ld_plugin_tv): Add new field.
> > >>
> > >> lto-plugin/ChangeLog:
> > >>
> > >> * lto-plugin.c (onload): Call ld_plugin_version.
> > >> ---
> > >> include/plugin-api.h| 18 +-
> > >> lto-plugin/lto-plugin.c | 4
> > >> 2 files changed, 21 insertions(+), 1 deletion(-)
> > >>
> > >> diff --git a/include/plugin-api.h b/include/plugin-api.h
> > >> index 4e12c0320d6..4d0989028cc 100644
> > >> --- a/include/plugin-api.h
> > >> +++ b/include/plugin-api.h
> > >> @@ -211,6 +211,13 @@ enum ld_plugin_symbol_section_kind
> > >>LDSSK_BSS
> > >> };
> > >>
> > >> +enum ld_plugin_compiler
> > >> +{
> > >> + LDPC_UNKNOWN,
> > >> + LDPC_GCC,
> > >> + LDPC_LLVM
> > >> +};
> > >> +
> > >> /* How a symbol is resolved. */
> > >>
> > >> enum ld_plugin_symbol_resolution
> > >> @@ -475,6 +482,13 @@ enum ld_plugin_status
> > >> (*ld_plugin_get_wrap_symbols) (uint64_t *num_symbols,
> > >> const char ***wrap_symbol_list);
> > >>
> > >> +/* The linker's interface for registering the "plugin_version" handler.
> > >> + This handler is called directly after onload and provides compiler
> > >> + and its number version (MAJOR * 1000 + MINOR). */
> > >> +
> > >> +typedef
> > >> +void (*ld_plugin_version) (enum ld_plugin_compiler compiler, int
> > >> version);
> > >> +
> > >> enum ld_plugin_level
> > >> {
> > >>LDPL_INFO,
> > >> @@ -520,7 +534,8 @@ enum ld_plugin_tag
> > >>LDPT_GET_INPUT_SECTION_SIZE = 30,
> > >>LDPT_REGISTER_NEW_INPUT_HOOK = 31,
> > >>LDPT_GET_WRAP_SYMBOLS = 32,
> > >> - LDPT_ADD_SYMBOLS_V2 = 33
> > >> + LDPT_ADD_SYMBOLS_V2 = 33,
> > >> + LDPT_PLUGIN_VERSION = 34,
> > >> };
> > >>
> > >> /* The plugin transfer vector. */
> > >> @@ -556,6 +571,7 @@ struct ld_plugin_tv
> > >> ld_plugin_get_input_section_size tv_get_input_section_size;
> > >> ld_plugin_register_new_input tv_register_new_input;
> > >> ld_plugin_get_wrap_symbols tv_get_wrap_symbols;
> > >> +ld_plugin_version tv_plugin_version;
> > >>} tv_u;
> > >> };
> > >>
> > >> diff --git a/lto-plugin/lto-plugin.c b/lto-plugin/lto-plugin.c
> > >> index 7a1c77812f6..698a0617ca7 100644
> > >> --- a/lto-plugin/lto-plugin.c
> > >> +++ b/lto-plugin/lto-plugin.c
> > >> @@ -69,6 +69,7 @@ along with this program; see the file COPYING3. If
> > >> not see
> > >> #include "../gcc/lto/common.h"
> > >> #include "simple-object.h"
> > >> #include "plugin-api.h"
> > >> +#include "ansidecl.h"
> > >>
> > >> /* We need to use I64 instead of ll width-specifier on native Windows.
> > >> The reason for this is that older MS-runtimes don't support the ll.
> > >> */
> > >> @@ -1466,6 +1467,9 @@ onload (struct ld_plugin_tv *tv)
> > >> /* We only use this to make user-friendly temp file names. */
> > >> link_output_name = p->tv_u.tv_string;
> > >> break;
> > >> + case LDPT_PLUGIN_VERSION:
> > >> +
Re: [PATCH] LTO plugin: add ld_plugin_version callback.
On Mon, May 2, 2022 at 10:19 AM Martin Liška wrote:
>
> On 5/2/22 10:09, Richard Biener wrote:
> > On Mon, May 2, 2022 at 9:52 AM Martin Liška wrote:
> >>
> >> Hi.
> >>
> >> This in a new plug-in function that helps identifying compiler
> >> by a linker. Will be used in mold linker.
> >>
> >> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> >>
> >> Ready to be installed?
> >
> > It looks a bit backward to query the compiler (and even more so to
> > have a fixed set) from the linker. What is this going to be used for?
>
> It's going to be used by mold, there are small behavior divergence
> in between LLVM and GCC, where one compiler closes provided file descriptors:
> https://github.com/rui314/mold/blob/a15f3875269b575d024725590ae3ef87b611b8d8/elf/lto.cc#L545-L550
But that then seems to be a defect in the plugin API specification (or
one of the two implementations). Instead of adding a new API entry
that defect should be fixed - both require changes in the actual plugin
specification.
Richard.
> > If then this should probably receive a unformatted string the linker
> > has to decipher itself like "gcc (SUSE Linux) 7.5.0" or something
> > (what we produce with gcc --version), and clang would produce
> > "clang version 11.0.1" or so?
>
> Well, it brings its own set of problems such string parsing. Note that the
> plugin API
> currently provides something like:
>
> /* The version of gold being used, or -1 if not gold. The number is
>MAJOR * 100 + MINOR. */
> static int gold_version = -1;
>
> So my suggestion is quite aligned with that.
>
> Martin
>
> >
> > Richard.
> >
> >> Thanks,
> >> Martin
> >>
> >> include/ChangeLog:
> >>
> >> * plugin-api.h (enum ld_plugin_compiler): New enum.
> >> (enum ld_plugin_version): New typedef.
> >> (struct ld_plugin_tv): Add new field.
> >>
> >> lto-plugin/ChangeLog:
> >>
> >> * lto-plugin.c (onload): Call ld_plugin_version.
> >> ---
> >> include/plugin-api.h| 18 +-
> >> lto-plugin/lto-plugin.c | 4
> >> 2 files changed, 21 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/include/plugin-api.h b/include/plugin-api.h
> >> index 4e12c0320d6..4d0989028cc 100644
> >> --- a/include/plugin-api.h
> >> +++ b/include/plugin-api.h
> >> @@ -211,6 +211,13 @@ enum ld_plugin_symbol_section_kind
> >>LDSSK_BSS
> >> };
> >>
> >> +enum ld_plugin_compiler
> >> +{
> >> + LDPC_UNKNOWN,
> >> + LDPC_GCC,
> >> + LDPC_LLVM
> >> +};
> >> +
> >> /* How a symbol is resolved. */
> >>
> >> enum ld_plugin_symbol_resolution
> >> @@ -475,6 +482,13 @@ enum ld_plugin_status
> >> (*ld_plugin_get_wrap_symbols) (uint64_t *num_symbols,
> >> const char ***wrap_symbol_list);
> >>
> >> +/* The linker's interface for registering the "plugin_version" handler.
> >> + This handler is called directly after onload and provides compiler
> >> + and its number version (MAJOR * 1000 + MINOR). */
> >> +
> >> +typedef
> >> +void (*ld_plugin_version) (enum ld_plugin_compiler compiler, int version);
> >> +
> >> enum ld_plugin_level
> >> {
> >>LDPL_INFO,
> >> @@ -520,7 +534,8 @@ enum ld_plugin_tag
> >>LDPT_GET_INPUT_SECTION_SIZE = 30,
> >>LDPT_REGISTER_NEW_INPUT_HOOK = 31,
> >>LDPT_GET_WRAP_SYMBOLS = 32,
> >> - LDPT_ADD_SYMBOLS_V2 = 33
> >> + LDPT_ADD_SYMBOLS_V2 = 33,
> >> + LDPT_PLUGIN_VERSION = 34,
> >> };
> >>
> >> /* The plugin transfer vector. */
> >> @@ -556,6 +571,7 @@ struct ld_plugin_tv
> >> ld_plugin_get_input_section_size tv_get_input_section_size;
> >> ld_plugin_register_new_input tv_register_new_input;
> >> ld_plugin_get_wrap_symbols tv_get_wrap_symbols;
> >> +ld_plugin_version tv_plugin_version;
> >>} tv_u;
> >> };
> >>
> >> diff --git a/lto-plugin/lto-plugin.c b/lto-plugin/lto-plugin.c
> >> index 7a1c77812f6..698a0617ca7 100644
> >> --- a/lto-plugin/lto-plugin.c
> >> +++ b/lto-plugin/lto-plugin.c
> >> @@ -69,6 +69,7 @@ along with this program; see the file COPYING3. If not
> >> see
> >> #include "../gcc/lto/common.h"
> >> #include "simple-object.h"
> >> #include "plugin-api.h"
> >> +#include "ansidecl.h"
> >>
> >> /* We need to use I64 instead of ll width-specifier on native Windows.
> >> The reason for this is that older MS-runtimes don't support the ll. */
> >> @@ -1466,6 +1467,9 @@ onload (struct ld_plugin_tv *tv)
> >> /* We only use this to make user-friendly temp file names. */
> >> link_output_name = p->tv_u.tv_string;
> >> break;
> >> + case LDPT_PLUGIN_VERSION:
> >> + p->tv_u.tv_plugin_version (LDPC_GCC, GCC_VERSION);
> >> + break;
> >> default:
> >> break;
> >> }
> >> --
> >> 2.36.0
> >>
>
Re: [PATCH] ipa-visibility: Optimize TLS access [PR99619]
Hi,
I'd like to ping this patch on behalf of Artem.
Alexander
On Sun, 17 Apr 2022, Artem Klimov wrote:
> Fix issue PR99619, which asks to optimize TLS access based on
> visibility. The fix is implemented as an IPA optimization, which allows
> to take optimized visibility status into account (as well as avoid
> modifying all language frontends).
>
> 2022-04-17 Artem Klimov
>
> gcc/ChangeLog:
> PR middle-end/99619
> * ipa-visibility.cc (function_and_variable_visibility): Add an
> explicit TLS model update after visibility optimisation loops.
>
> gcc/testsuite/ChangeLog:
> PR middle-end/99619
> * gcc.dg/tls/vis-attr-gd.c: New test.
> * gcc.dg/tls/vis-attr-hidden-gd.c: New test.
> * gcc.dg/tls/vis-attr-hidden.c: New test.
> * gcc.dg/tls/vis-flag-hidden-gd.c: New test.
> * gcc.dg/tls/vis-flag-hidden.c: New test.
> * gcc.dg/tls/vis-pragma-hidden-gd.c: New test.
> * gcc.dg/tls/vis-pragma-hidden.c: New test.
>
> Co-Authored-By: Alexander Monakov
> Signed-off-by: Artem Klimov
> ---
> gcc/ipa-visibility.cc | 16
> gcc/testsuite/gcc.dg/tls/vis-attr-gd.c | 10 ++
> gcc/testsuite/gcc.dg/tls/vis-attr-hidden-gd.c | 11 +++
> gcc/testsuite/gcc.dg/tls/vis-attr-hidden.c | 10 ++
> gcc/testsuite/gcc.dg/tls/vis-flag-hidden-gd.c | 11 +++
> gcc/testsuite/gcc.dg/tls/vis-flag-hidden.c | 10 ++
> gcc/testsuite/gcc.dg/tls/vis-pragma-hidden-gd.c | 15 +++
> gcc/testsuite/gcc.dg/tls/vis-pragma-hidden.c| 14 ++
> 8 files changed, 97 insertions(+)
> create mode 100644 gcc/testsuite/gcc.dg/tls/vis-attr-gd.c
> create mode 100644 gcc/testsuite/gcc.dg/tls/vis-attr-hidden-gd.c
> create mode 100644 gcc/testsuite/gcc.dg/tls/vis-attr-hidden.c
> create mode 100644 gcc/testsuite/gcc.dg/tls/vis-flag-hidden-gd.c
> create mode 100644 gcc/testsuite/gcc.dg/tls/vis-flag-hidden.c
> create mode 100644 gcc/testsuite/gcc.dg/tls/vis-pragma-hidden-gd.c
> create mode 100644 gcc/testsuite/gcc.dg/tls/vis-pragma-hidden.c
>
> diff --git a/gcc/ipa-visibility.cc b/gcc/ipa-visibility.cc
> index e95a0dd252f..ca5b9a95f5e 100644
> --- a/gcc/ipa-visibility.cc
> +++ b/gcc/ipa-visibility.cc
> @@ -872,6 +872,22 @@ function_and_variable_visibility (bool whole_program)
> }
> }
> }
> + FOR_EACH_VARIABLE (vnode)
> +{
> + tree decl = vnode->decl;
> +
> + /* Optimize TLS model based on visibility (taking into account
> + optimizations done in the preceding loop), unless it was
> + specified explicitly. */
> +
> + if (DECL_THREAD_LOCAL_P (decl)
> + && !lookup_attribute ("tls_model", DECL_ATTRIBUTES (decl)))
> +{
> + enum tls_model new_model = decl_default_tls_model (decl);
> + gcc_checking_assert (new_model >= decl_tls_model (decl));
> + set_decl_tls_model (decl, new_model);
> +}
> +}
>
>if (dump_file)
> {
> diff --git a/gcc/testsuite/gcc.dg/tls/vis-attr-gd.c
> b/gcc/testsuite/gcc.dg/tls/vis-attr-gd.c
> new file mode 100644
> index 000..473c7846f74
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tls/vis-attr-gd.c
> @@ -0,0 +1,10 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target fpic } */
> +/* { dg-require-effective-target tls } */
> +/* { dg-options "-O2 -fPIC -fdump-ipa-visibility" } */
> +
> +// tls_model should be global-dynamic due to explicitly specified attribute
> +__attribute__((tls_model("global-dynamic")))
> +__thread int x;
> +
> +/* { dg-final { scan-ipa-dump "Varpool flags: tls-global-dynamic"
> "visibility" } } */
> diff --git a/gcc/testsuite/gcc.dg/tls/vis-attr-hidden-gd.c
> b/gcc/testsuite/gcc.dg/tls/vis-attr-hidden-gd.c
> new file mode 100644
> index 000..8f592052361
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tls/vis-attr-hidden-gd.c
> @@ -0,0 +1,11 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target fpic } */
> +/* { dg-require-effective-target tls } */
> +/* { dg-options "-O2 -fPIC -fdump-ipa-visibility" } */
> +
> +// tls_model should be global-dynamic due to explicitly specified attribute
> +__attribute__((visibility("hidden")))
> +__attribute__((tls_model("global-dynamic")))
> +__thread int x;
> +
> +/* { dg-final { scan-ipa-dump "Varpool flags: tls-global-dynamic"
> "visibility" } } */
> diff --git a/gcc/testsuite/gcc.dg/tls/vis-attr-hidden.c
> b/gcc/testsuite/gcc.dg/tls/vis-attr-hidden.c
> new file mode 100644
> index 000..2da1bc3fa42
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tls/vis-attr-hidden.c
> @@ -0,0 +1,10 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target fpic } */
> +/* { dg-require-effective-target tls } */
> +/* { dg-options "-O2 -fPIC -fdump-ipa-visibility" } */
> +
> +//tls_model should be local-dynamic due to visibility("hidden")
> +__attribute__((visibility("hidden")))
> +__thread int x;
> +
> +/* { dg-final
Re: [PATCH] gcov: Fix first time gcov info dump
On 5/2/22 10:43, Sebastian Huber wrote:
> This patch fixes an issue introduced by commit
> ef9a53feae5701953da9161afef2aea0329ec8b2:
Works for me, please install it.
Martin
>
> gcc --coverage main.c && ./a.out
> libgcov profiling error:a-main.gcda:Error writing
>
> gcc/ChangeLog:
>
> * gcov-io.cc (gcov_rewrite): Clear the file error status.
> ---
> gcc/gcov-io.cc | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/gcov-io.cc b/gcc/gcov-io.cc
> index fdf745e6ce1..62032ccfa18 100644
> --- a/gcc/gcov-io.cc
> +++ b/gcc/gcov-io.cc
> @@ -79,11 +79,14 @@ gcov_is_error (void)
> }
>
> #if IN_LIBGCOV
> -/* Move to beginning of file and initialize for writing. */
> +/* Move to beginning of file, initialize for writing, and clear file error
> + status. */
> +
> GCOV_LINKAGE inline void
> gcov_rewrite (void)
> {
>gcov_var.mode = -1;
> + gcov_var.error = GCOV_FILE_NO_ERROR;
>fseek (gcov_var.file, 0L, SEEK_SET);
> }
> #endif
[PATCH] tree-optimization/105437 - BB vect with extern defs of throwing stmts
We have to watch out for vectorized stmt insert locations if the
def from the last stmt alters control flow. We constrain region
building so we know the def is outside of the current region
and thus we can insert at the region start point.
Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed to trunk
sofar. If we do RC2 then I'd consider this there as well as it is
exposed by us enabling vectorization at -O2 by default in GCC 12.
Richard.
2022-05-02 Richard Biener
PR tree-optimization/105437
* tree-vect-slp.cc (vect_schedule_slp_node): Handle the
case where last_stmt alters control flow.
* g++.dg/vect/pr105437.cc: New testcase.
---
gcc/testsuite/g++.dg/vect/pr105437.cc | 24
gcc/tree-vect-slp.cc | 7 +++
2 files changed, 31 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/vect/pr105437.cc
diff --git a/gcc/testsuite/g++.dg/vect/pr105437.cc
b/gcc/testsuite/g++.dg/vect/pr105437.cc
new file mode 100644
index 000..b3b440debef
--- /dev/null
+++ b/gcc/testsuite/g++.dg/vect/pr105437.cc
@@ -0,0 +1,24 @@
+// { dg-do compile }
+
+struct ControlClass
+{
+virtual ~ControlClass();
+
+int Width;
+int Height;
+unsigned IsToRepaint : 1;
+};
+
+struct SelectClass : ControlClass
+{
+SelectClass(void);
+};
+
+int Non_Folded_Value();
+
+SelectClass::SelectClass(void)
+{
+int factor = Non_Folded_Value();
+Width = 32 << factor;
+Height = 24 << factor;
+}
diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc
index 805dd7e10e2..0d400c00df1 100644
--- a/gcc/tree-vect-slp.cc
+++ b/gcc/tree-vect-slp.cc
@@ -7302,6 +7302,13 @@ vect_schedule_slp_node (vec_info *vinfo,
gcc_assert (seen_vector_def);
si = gsi_after_labels (as_a (vinfo)->bbs[0]);
}
+ else if (is_ctrl_altering_stmt (last_stmt))
+ {
+ /* We split regions to vectorize at control altering stmts
+with a definition so this must be an external which
+we can insert at the start of the region. */
+ si = gsi_after_labels (as_a (vinfo)->bbs[0]);
+ }
else if (is_a (vinfo)
&& gimple_bb (last_stmt) != gimple_bb (stmt_info->stmt)
&& gimple_could_trap_p (stmt_info->stmt))
--
2.34.1
[PATCH] gcov: Fix first time gcov info dump
This patch fixes an issue introduced by commit
ef9a53feae5701953da9161afef2aea0329ec8b2:
gcc --coverage main.c && ./a.out
libgcov profiling error:a-main.gcda:Error writing
gcc/ChangeLog:
* gcov-io.cc (gcov_rewrite): Clear the file error status.
---
gcc/gcov-io.cc | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/gcc/gcov-io.cc b/gcc/gcov-io.cc
index fdf745e6ce1..62032ccfa18 100644
--- a/gcc/gcov-io.cc
+++ b/gcc/gcov-io.cc
@@ -79,11 +79,14 @@ gcov_is_error (void)
}
#if IN_LIBGCOV
-/* Move to beginning of file and initialize for writing. */
+/* Move to beginning of file, initialize for writing, and clear file error
+ status. */
+
GCOV_LINKAGE inline void
gcov_rewrite (void)
{
gcov_var.mode = -1;
+ gcov_var.error = GCOV_FILE_NO_ERROR;
fseek (gcov_var.file, 0L, SEEK_SET);
}
#endif
--
2.34.1
Re: [PATCH] alias: Fix -fcompare-debug issues caused by compare_base_symbol_refs [PR105415]
On Mon, 2 May 2022, Jakub Jelinek wrote:
> On Fri, Apr 29, 2022 at 01:23:46PM +0200, Richard Biener wrote:
> > On Fri, 29 Apr 2022, Jakub Jelinek wrote:
> >
> > > On Fri, Apr 29, 2022 at 01:11:31PM +0200, Jakub Jelinek via Gcc-patches
> > > wrote:
> > > > Depends. DECL_IN_CONSTANT_POOL decls can appear 2 ways, through
> > > > tree_output_constant_def which does create a varpool node for them
> > > > and is generally invoked during GIMPLE passes or so, and using
> > > > output_constant_def, which is called during expansion or later and
> > > > doesn't
> > > > have varpool nodes created unless say alias.cc creates those for them.
> > >
> > > Oh, and one thing I forgot. The constant pool decls can be put into
> > > section
> > > anchors, so it is essential that we handle DECL_IN_CONSTANT_POOL decls
> > > there and don't just punt on those.
> >
> > Ah, OK - that makes sense (maybe we should create varpool nodes at the
> > point we associate them with anchors, or alternatively use the varpool
> > node of the anchor?).
>
> I've bootstrapped/regtested the following on x86_64-linux and i686-linux,
> then bootstrapped with the patch reverted, reapplied the patch and did make
> cc1plus in stage3. The debug section sizes are identical, .debug_info and
> .debug_loc is identical too, so I think we don't lose any debug info through
> it.
>
> Ok for trunk?
OK.
Richard.
> 2022-05-02 Jakub Jelinek
>
> PR debug/105415
> * cfgexpand.cc (expand_debug_expr): Don't make_decl_rtl_for_debug
> if there is no symtab node for the VAR_DECL.
>
> * gcc.dg/pr105415.c: New test.
>
> --- gcc/cfgexpand.cc.jj 2022-03-09 19:54:17.112284770 +0100
> +++ gcc/cfgexpand.cc 2022-04-29 12:33:32.585363999 +0200
> @@ -4565,7 +4565,8 @@ expand_debug_expr (tree exp)
> || !DECL_NAME (exp)
> || DECL_HARD_REGISTER (exp)
> || DECL_IN_CONSTANT_POOL (exp)
> - || mode == VOIDmode)
> + || mode == VOIDmode
> + || symtab_node::get (exp) == NULL)
> return NULL;
>
> op0 = make_decl_rtl_for_debug (exp);
> --- gcc/testsuite/gcc.dg/pr105415.c.jj2022-04-28 12:26:13.174302870
> +0200
> +++ gcc/testsuite/gcc.dg/pr105415.c 2022-04-28 12:25:36.770809149 +0200
> @@ -0,0 +1,26 @@
> +/* PR debug/105415 */
> +/* { dg-do compile } */
> +/* { dg-require-effective-target pthread } */
> +/* { dg-options "-O2 -ftree-parallelize-loops=2 -fcompare-debug" } */
> +
> +int m;
> +static int n;
> +
> +void
> +foo (void)
> +{
> + int s = 0;
> +
> + while (m < 1)
> +{
> + s += n;
> + ++m;
> +}
> +}
> +
> +void
> +bar (int *arr, int i)
> +{
> + while (i < 1)
> +arr[i++] = 1;
> +}
>
>
> Jakub
>
>
--
Richard Biener
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)
Re: [PATCH] alias: Fix -fcompare-debug issues caused by compare_base_symbol_refs [PR105415]
On Fri, Apr 29, 2022 at 01:23:46PM +0200, Richard Biener wrote:
> On Fri, 29 Apr 2022, Jakub Jelinek wrote:
>
> > On Fri, Apr 29, 2022 at 01:11:31PM +0200, Jakub Jelinek via Gcc-patches
> > wrote:
> > > Depends. DECL_IN_CONSTANT_POOL decls can appear 2 ways, through
> > > tree_output_constant_def which does create a varpool node for them
> > > and is generally invoked during GIMPLE passes or so, and using
> > > output_constant_def, which is called during expansion or later and doesn't
> > > have varpool nodes created unless say alias.cc creates those for them.
> >
> > Oh, and one thing I forgot. The constant pool decls can be put into section
> > anchors, so it is essential that we handle DECL_IN_CONSTANT_POOL decls
> > there and don't just punt on those.
>
> Ah, OK - that makes sense (maybe we should create varpool nodes at the
> point we associate them with anchors, or alternatively use the varpool
> node of the anchor?).
I've bootstrapped/regtested the following on x86_64-linux and i686-linux,
then bootstrapped with the patch reverted, reapplied the patch and did make
cc1plus in stage3. The debug section sizes are identical, .debug_info and
.debug_loc is identical too, so I think we don't lose any debug info through
it.
Ok for trunk?
2022-05-02 Jakub Jelinek
PR debug/105415
* cfgexpand.cc (expand_debug_expr): Don't make_decl_rtl_for_debug
if there is no symtab node for the VAR_DECL.
* gcc.dg/pr105415.c: New test.
--- gcc/cfgexpand.cc.jj 2022-03-09 19:54:17.112284770 +0100
+++ gcc/cfgexpand.cc2022-04-29 12:33:32.585363999 +0200
@@ -4565,7 +4565,8 @@ expand_debug_expr (tree exp)
|| !DECL_NAME (exp)
|| DECL_HARD_REGISTER (exp)
|| DECL_IN_CONSTANT_POOL (exp)
- || mode == VOIDmode)
+ || mode == VOIDmode
+ || symtab_node::get (exp) == NULL)
return NULL;
op0 = make_decl_rtl_for_debug (exp);
--- gcc/testsuite/gcc.dg/pr105415.c.jj 2022-04-28 12:26:13.174302870 +0200
+++ gcc/testsuite/gcc.dg/pr105415.c 2022-04-28 12:25:36.770809149 +0200
@@ -0,0 +1,26 @@
+/* PR debug/105415 */
+/* { dg-do compile } */
+/* { dg-require-effective-target pthread } */
+/* { dg-options "-O2 -ftree-parallelize-loops=2 -fcompare-debug" } */
+
+int m;
+static int n;
+
+void
+foo (void)
+{
+ int s = 0;
+
+ while (m < 1)
+{
+ s += n;
+ ++m;
+}
+}
+
+void
+bar (int *arr, int i)
+{
+ while (i < 1)
+arr[i++] = 1;
+}
Jakub
Re: [PATCH] LTO plugin: add ld_plugin_version callback.
On 5/2/22 10:09, Richard Biener wrote:
> On Mon, May 2, 2022 at 9:52 AM Martin Liška wrote:
>>
>> Hi.
>>
>> This in a new plug-in function that helps identifying compiler
>> by a linker. Will be used in mold linker.
>>
>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>
>> Ready to be installed?
>
> It looks a bit backward to query the compiler (and even more so to
> have a fixed set) from the linker. What is this going to be used for?
It's going to be used by mold, there are small behavior divergence
in between LLVM and GCC, where one compiler closes provided file descriptors:
https://github.com/rui314/mold/blob/a15f3875269b575d024725590ae3ef87b611b8d8/elf/lto.cc#L545-L550
> If then this should probably receive a unformatted string the linker
> has to decipher itself like "gcc (SUSE Linux) 7.5.0" or something
> (what we produce with gcc --version), and clang would produce
> "clang version 11.0.1" or so?
Well, it brings its own set of problems such string parsing. Note that the
plugin API
currently provides something like:
/* The version of gold being used, or -1 if not gold. The number is
MAJOR * 100 + MINOR. */
static int gold_version = -1;
So my suggestion is quite aligned with that.
Martin
>
> Richard.
>
>> Thanks,
>> Martin
>>
>> include/ChangeLog:
>>
>> * plugin-api.h (enum ld_plugin_compiler): New enum.
>> (enum ld_plugin_version): New typedef.
>> (struct ld_plugin_tv): Add new field.
>>
>> lto-plugin/ChangeLog:
>>
>> * lto-plugin.c (onload): Call ld_plugin_version.
>> ---
>> include/plugin-api.h| 18 +-
>> lto-plugin/lto-plugin.c | 4
>> 2 files changed, 21 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/plugin-api.h b/include/plugin-api.h
>> index 4e12c0320d6..4d0989028cc 100644
>> --- a/include/plugin-api.h
>> +++ b/include/plugin-api.h
>> @@ -211,6 +211,13 @@ enum ld_plugin_symbol_section_kind
>>LDSSK_BSS
>> };
>>
>> +enum ld_plugin_compiler
>> +{
>> + LDPC_UNKNOWN,
>> + LDPC_GCC,
>> + LDPC_LLVM
>> +};
>> +
>> /* How a symbol is resolved. */
>>
>> enum ld_plugin_symbol_resolution
>> @@ -475,6 +482,13 @@ enum ld_plugin_status
>> (*ld_plugin_get_wrap_symbols) (uint64_t *num_symbols,
>> const char ***wrap_symbol_list);
>>
>> +/* The linker's interface for registering the "plugin_version" handler.
>> + This handler is called directly after onload and provides compiler
>> + and its number version (MAJOR * 1000 + MINOR). */
>> +
>> +typedef
>> +void (*ld_plugin_version) (enum ld_plugin_compiler compiler, int version);
>> +
>> enum ld_plugin_level
>> {
>>LDPL_INFO,
>> @@ -520,7 +534,8 @@ enum ld_plugin_tag
>>LDPT_GET_INPUT_SECTION_SIZE = 30,
>>LDPT_REGISTER_NEW_INPUT_HOOK = 31,
>>LDPT_GET_WRAP_SYMBOLS = 32,
>> - LDPT_ADD_SYMBOLS_V2 = 33
>> + LDPT_ADD_SYMBOLS_V2 = 33,
>> + LDPT_PLUGIN_VERSION = 34,
>> };
>>
>> /* The plugin transfer vector. */
>> @@ -556,6 +571,7 @@ struct ld_plugin_tv
>> ld_plugin_get_input_section_size tv_get_input_section_size;
>> ld_plugin_register_new_input tv_register_new_input;
>> ld_plugin_get_wrap_symbols tv_get_wrap_symbols;
>> +ld_plugin_version tv_plugin_version;
>>} tv_u;
>> };
>>
>> diff --git a/lto-plugin/lto-plugin.c b/lto-plugin/lto-plugin.c
>> index 7a1c77812f6..698a0617ca7 100644
>> --- a/lto-plugin/lto-plugin.c
>> +++ b/lto-plugin/lto-plugin.c
>> @@ -69,6 +69,7 @@ along with this program; see the file COPYING3. If not see
>> #include "../gcc/lto/common.h"
>> #include "simple-object.h"
>> #include "plugin-api.h"
>> +#include "ansidecl.h"
>>
>> /* We need to use I64 instead of ll width-specifier on native Windows.
>> The reason for this is that older MS-runtimes don't support the ll. */
>> @@ -1466,6 +1467,9 @@ onload (struct ld_plugin_tv *tv)
>> /* We only use this to make user-friendly temp file names. */
>> link_output_name = p->tv_u.tv_string;
>> break;
>> + case LDPT_PLUGIN_VERSION:
>> + p->tv_u.tv_plugin_version (LDPC_GCC, GCC_VERSION);
>> + break;
>> default:
>> break;
>> }
>> --
>> 2.36.0
>>
Re: [PATCH] LTO plugin: add ld_plugin_version callback.
On Mon, May 2, 2022 at 9:52 AM Martin Liška wrote:
>
> Hi.
>
> This in a new plug-in function that helps identifying compiler
> by a linker. Will be used in mold linker.
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ready to be installed?
It looks a bit backward to query the compiler (and even more so to
have a fixed set) from the linker. What is this going to be used for?
If then this should probably receive a unformatted string the linker
has to decipher itself like "gcc (SUSE Linux) 7.5.0" or something
(what we produce with gcc --version), and clang would produce
"clang version 11.0.1" or so?
Richard.
> Thanks,
> Martin
>
> include/ChangeLog:
>
> * plugin-api.h (enum ld_plugin_compiler): New enum.
> (enum ld_plugin_version): New typedef.
> (struct ld_plugin_tv): Add new field.
>
> lto-plugin/ChangeLog:
>
> * lto-plugin.c (onload): Call ld_plugin_version.
> ---
> include/plugin-api.h| 18 +-
> lto-plugin/lto-plugin.c | 4
> 2 files changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/include/plugin-api.h b/include/plugin-api.h
> index 4e12c0320d6..4d0989028cc 100644
> --- a/include/plugin-api.h
> +++ b/include/plugin-api.h
> @@ -211,6 +211,13 @@ enum ld_plugin_symbol_section_kind
>LDSSK_BSS
> };
>
> +enum ld_plugin_compiler
> +{
> + LDPC_UNKNOWN,
> + LDPC_GCC,
> + LDPC_LLVM
> +};
> +
> /* How a symbol is resolved. */
>
> enum ld_plugin_symbol_resolution
> @@ -475,6 +482,13 @@ enum ld_plugin_status
> (*ld_plugin_get_wrap_symbols) (uint64_t *num_symbols,
> const char ***wrap_symbol_list);
>
> +/* The linker's interface for registering the "plugin_version" handler.
> + This handler is called directly after onload and provides compiler
> + and its number version (MAJOR * 1000 + MINOR). */
> +
> +typedef
> +void (*ld_plugin_version) (enum ld_plugin_compiler compiler, int version);
> +
> enum ld_plugin_level
> {
>LDPL_INFO,
> @@ -520,7 +534,8 @@ enum ld_plugin_tag
>LDPT_GET_INPUT_SECTION_SIZE = 30,
>LDPT_REGISTER_NEW_INPUT_HOOK = 31,
>LDPT_GET_WRAP_SYMBOLS = 32,
> - LDPT_ADD_SYMBOLS_V2 = 33
> + LDPT_ADD_SYMBOLS_V2 = 33,
> + LDPT_PLUGIN_VERSION = 34,
> };
>
> /* The plugin transfer vector. */
> @@ -556,6 +571,7 @@ struct ld_plugin_tv
> ld_plugin_get_input_section_size tv_get_input_section_size;
> ld_plugin_register_new_input tv_register_new_input;
> ld_plugin_get_wrap_symbols tv_get_wrap_symbols;
> +ld_plugin_version tv_plugin_version;
>} tv_u;
> };
>
> diff --git a/lto-plugin/lto-plugin.c b/lto-plugin/lto-plugin.c
> index 7a1c77812f6..698a0617ca7 100644
> --- a/lto-plugin/lto-plugin.c
> +++ b/lto-plugin/lto-plugin.c
> @@ -69,6 +69,7 @@ along with this program; see the file COPYING3. If not see
> #include "../gcc/lto/common.h"
> #include "simple-object.h"
> #include "plugin-api.h"
> +#include "ansidecl.h"
>
> /* We need to use I64 instead of ll width-specifier on native Windows.
> The reason for this is that older MS-runtimes don't support the ll. */
> @@ -1466,6 +1467,9 @@ onload (struct ld_plugin_tv *tv)
> /* We only use this to make user-friendly temp file names. */
> link_output_name = p->tv_u.tv_string;
> break;
> + case LDPT_PLUGIN_VERSION:
> + p->tv_u.tv_plugin_version (LDPC_GCC, GCC_VERSION);
> + break;
> default:
> break;
> }
> --
> 2.36.0
>
Re: [PATCH] i386: simplify cpu_feature handling
On 3/31/22 09:01, Martin Liška wrote: > @Jakub: May I install it once stage1 opens? May I please ping this? Thanks, Martin > > Cheers, > Martin > > On 1/3/22 12:43, Martin Liška wrote: >> PING: Jakub? >> >> On 12/15/21 10:57, Martin Liška wrote: >>> On 12/14/21 17:12, Jakub Jelinek wrote: I'd use INT_TYPE_SIZE - 1 instead of 31. Otherwise LGTM. >>> >>> Installed with that change, thanks. >>> >>> Moreover, I'm suggesting a simplification: >>> >>> The patch removes unneeded loops for cpu_features2 and CONVERT_EXPR >>> that can be simplified with NOP_EXPR. >>> >>> Survives i386.exp tests, may I install the patch after testing or >>> is it a stage1 material? >>> >>> Thanks, >>> Martin >> >
[PATCH] LTO plugin: add ld_plugin_version callback.
Hi.
This in a new plug-in function that helps identifying compiler
by a linker. Will be used in mold linker.
Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
Ready to be installed?
Thanks,
Martin
include/ChangeLog:
* plugin-api.h (enum ld_plugin_compiler): New enum.
(enum ld_plugin_version): New typedef.
(struct ld_plugin_tv): Add new field.
lto-plugin/ChangeLog:
* lto-plugin.c (onload): Call ld_plugin_version.
---
include/plugin-api.h| 18 +-
lto-plugin/lto-plugin.c | 4
2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/include/plugin-api.h b/include/plugin-api.h
index 4e12c0320d6..4d0989028cc 100644
--- a/include/plugin-api.h
+++ b/include/plugin-api.h
@@ -211,6 +211,13 @@ enum ld_plugin_symbol_section_kind
LDSSK_BSS
};
+enum ld_plugin_compiler
+{
+ LDPC_UNKNOWN,
+ LDPC_GCC,
+ LDPC_LLVM
+};
+
/* How a symbol is resolved. */
enum ld_plugin_symbol_resolution
@@ -475,6 +482,13 @@ enum ld_plugin_status
(*ld_plugin_get_wrap_symbols) (uint64_t *num_symbols,
const char ***wrap_symbol_list);
+/* The linker's interface for registering the "plugin_version" handler.
+ This handler is called directly after onload and provides compiler
+ and its number version (MAJOR * 1000 + MINOR). */
+
+typedef
+void (*ld_plugin_version) (enum ld_plugin_compiler compiler, int version);
+
enum ld_plugin_level
{
LDPL_INFO,
@@ -520,7 +534,8 @@ enum ld_plugin_tag
LDPT_GET_INPUT_SECTION_SIZE = 30,
LDPT_REGISTER_NEW_INPUT_HOOK = 31,
LDPT_GET_WRAP_SYMBOLS = 32,
- LDPT_ADD_SYMBOLS_V2 = 33
+ LDPT_ADD_SYMBOLS_V2 = 33,
+ LDPT_PLUGIN_VERSION = 34,
};
/* The plugin transfer vector. */
@@ -556,6 +571,7 @@ struct ld_plugin_tv
ld_plugin_get_input_section_size tv_get_input_section_size;
ld_plugin_register_new_input tv_register_new_input;
ld_plugin_get_wrap_symbols tv_get_wrap_symbols;
+ld_plugin_version tv_plugin_version;
} tv_u;
};
diff --git a/lto-plugin/lto-plugin.c b/lto-plugin/lto-plugin.c
index 7a1c77812f6..698a0617ca7 100644
--- a/lto-plugin/lto-plugin.c
+++ b/lto-plugin/lto-plugin.c
@@ -69,6 +69,7 @@ along with this program; see the file COPYING3. If not see
#include "../gcc/lto/common.h"
#include "simple-object.h"
#include "plugin-api.h"
+#include "ansidecl.h"
/* We need to use I64 instead of ll width-specifier on native Windows.
The reason for this is that older MS-runtimes don't support the ll. */
@@ -1466,6 +1467,9 @@ onload (struct ld_plugin_tv *tv)
/* We only use this to make user-friendly temp file names. */
link_output_name = p->tv_u.tv_string;
break;
+ case LDPT_PLUGIN_VERSION:
+ p->tv_u.tv_plugin_version (LDPC_GCC, GCC_VERSION);
+ break;
default:
break;
}
--
2.36.0
[PATCH] Support LDPT_GET_SYMBOLS_V3.
That supports skipping of an object file (LDPS_NO_SYMS). The API will be used
by mold
and is there for some time in linkers.
Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
Ready to be installed?
Thanks,
Martin
lto-plugin/ChangeLog:
* lto-plugin.c (struct plugin_file_info): Add skip_file flag.
(write_resolution): Write resolution only if get_symbols !=
LDPS_NO_SYMS.
(all_symbols_read_handler): Ignore file if skip_file is true.
(onload): Handle LDPT_GET_SYMBOLS_V3.
---
lto-plugin/lto-plugin.c | 42 ++---
1 file changed, 35 insertions(+), 7 deletions(-)
diff --git a/lto-plugin/lto-plugin.c b/lto-plugin/lto-plugin.c
index 33d49571d0e..aa5eab48b9d 100644
--- a/lto-plugin/lto-plugin.c
+++ b/lto-plugin/lto-plugin.c
@@ -136,6 +136,7 @@ struct plugin_file_info
void *handle;
struct plugin_symtab symtab;
struct plugin_symtab conflicts;
+ bool skip_file;
};
/* List item with name of the file with offloading. */
@@ -159,7 +160,7 @@ enum symbol_style
static char *arguments_file_name;
static ld_plugin_register_claim_file register_claim_file;
static ld_plugin_register_all_symbols_read register_all_symbols_read;
-static ld_plugin_get_symbols get_symbols, get_symbols_v2;
+static ld_plugin_get_symbols get_symbols, get_symbols_v2, get_symbols_v3;
static ld_plugin_register_cleanup register_cleanup;
static ld_plugin_add_input_file add_input_file;
static ld_plugin_add_input_library add_input_library;
@@ -547,15 +548,13 @@ free_symtab (struct plugin_symtab *symtab)
static void
write_resolution (void)
{
- unsigned int i;
+ unsigned int i, included_files = 0;
FILE *f;
check (resolution_file, LDPL_FATAL, "resolution file not specified");
f = fopen (resolution_file, "w");
check (f, LDPL_FATAL, "could not open file");
- fprintf (f, "%d\n", num_claimed_files);
-
for (i = 0; i < num_claimed_files; i++)
{
struct plugin_file_info *info = _files[i];
@@ -563,13 +562,38 @@ write_resolution (void)
struct ld_plugin_symbol *syms = symtab->syms;
/* Version 2 of API supports IRONLY_EXP resolution that is
- accepted by GCC-4.7 and newer. */
- if (get_symbols_v2)
+accepted by GCC-4.7 and newer.
+Version 3 can return LDPS_NO_SYMS that means the object
+will not be used at all. */
+ if (get_symbols_v3)
+ {
+ enum ld_plugin_status status
+ = get_symbols_v3 (info->handle, symtab->nsyms, syms);
+ if (status == LDPS_NO_SYMS)
+ {
+ info->skip_file = true;
+ continue;
+ }
+ }
+ else if (get_symbols_v2)
get_symbols_v2 (info->handle, symtab->nsyms, syms);
else
get_symbols (info->handle, symtab->nsyms, syms);
+ ++included_files;
+
finish_conflict_resolution (symtab, >conflicts);
+}
+
+ fprintf (f, "%d\n", included_files);
+
+ for (i = 0; i < num_claimed_files; i++)
+{
+ struct plugin_file_info *info = _files[i];
+ struct plugin_symtab *symtab = >symtab;
+
+ if (info->skip_file)
+ continue;
fprintf (f, "%s %d\n", info->name, symtab->nsyms +
info->conflicts.nsyms);
dump_symtab (f, symtab);
@@ -832,7 +856,8 @@ all_symbols_read_handler (void)
{
struct plugin_file_info *info = _files[i];
- *lto_arg_ptr++ = info->name;
+ if (!info->skip_file)
+ *lto_arg_ptr++ = info->name;
}
*lto_arg_ptr++ = NULL;
@@ -1409,6 +1434,9 @@ onload (struct ld_plugin_tv *tv)
case LDPT_REGISTER_ALL_SYMBOLS_READ_HOOK:
register_all_symbols_read = p->tv_u.tv_register_all_symbols_read;
break;
+ case LDPT_GET_SYMBOLS_V3:
+ get_symbols_v3 = p->tv_u.tv_get_symbols;
+ break;
case LDPT_GET_SYMBOLS_V2:
get_symbols_v2 = p->tv_u.tv_get_symbols;
break;
--
2.36.0
Re: [PATCH] Replace EVRP in DOM with ranger.
On Mon, May 2, 2022 at 8:30 AM Richard Biener
wrote:
>
> On Fri, Apr 29, 2022 at 6:22 PM Aldy Hernandez wrote:
> >
> > On Fri, Apr 29, 2022 at 12:13 PM Richard Biener
> > wrote:
> > >
> > > On Fri, Apr 29, 2022 at 11:53 AM Aldy Hernandez wrote:
> > > >
> > > > On Fri, Apr 29, 2022 at 9:09 AM Richard Biener
> > > > wrote:
> > > > >
> > > > > On Thu, Apr 28, 2022 at 8:44 PM Jeff Law via Gcc-patches
> > > > > wrote:
> > > > > >
> > > > > >
> > > > > >
> > > > > > On 4/28/2022 10:30 AM, Aldy Hernandez wrote:
> > > > > > > [Jeff, this is the same patch I sent you last week with minor
> > > > > > > tweaks
> > > > > > > to the commit message.]
> > > > > > And I just dropped it into the tester. We should have the vast
> > > > > > majority
> > > > > > of targets tested by this time tomorrow.
> > > > > >
> > > > > > >
> > > > > > > [Despite the verbosity of the message, this is actually a pretty
> > > > > > > straightforward patch. It should've gone in last cycle, but there
> > > > > > > was a nagging regression I couldn't get to until after stage1
> > > > > > > had closed.]
> > > > > > You had other, non-work/gcc priorities. No worries at missing
> > > > > > gcc-12.
> > > > > >
> > > > > >
> > > > > > >
> > > > > > > There are 3 uses of EVRP in DOM that must be converted.
> > > > > > > Unfortunately, they need to be converted in one go, so further
> > > > > > > splitting of this patch would be problematic.
> > > > > > ACK
> > > > > >
> > > > > >
> > > > > > > There's nothing here earth shattering. It's all pretty obvious in
> > > > > > > retrospect, but I've added a short description of each use to aid
> > > > > > > in
> > > > > > > reviewing:
> > > > > > >
> > > > > > > * Convert evrp use in cprop to ranger.
> > > > > > >
> > > > > > >This is easy, as cprop in DOM was converted to the ranger API
> > > > > > > last
> > > > > > >cycle, so this is just a matter of using a ranger instead of an
> > > > > > >evrp_range_analyzer.
> > > > > > Yea. We may have covered this before, but what does ranger do with
> > > > > > a
> > > > > > conditional equivalence?
> > > > > >
> > > > > > ie if we have something like
> > > > > >
> > > > > > if (a == b)
> > > > > >{
> > > > > > blah blah
> > > > > >}
> > > > > >
> > > > > > Within the true arm we know there's an equivalence. *But* we don't
> > > > > > want
> > > > > > to just blindly replace a with b or vice-versa. The equivalence is
> > > > > > primarily useful for simplification rather than propagation.
> > > > > >
> > > > > > In fact, we every much do not want to cprop in these cases. It
> > > > > > rarely
> > > > > > helps in a meaningful way and there's no good way to know which is
> > > > > > the
> > > > > > better value to use. Canonicalization on SSA_NAMEs doesn't really
> > > > > > help
> > > > > > due to SSA_NAME recycling.
> > > > > >
> > > > > >
> > > > > > >
> > > > > > > * Convert evrp use in threader to ranger.
> > > > > > >
> > > > > > >The idea here is to use the hybrid approach we used for the
> > > > > > > initial
> > > > > > >VRP threader conversion last cycle. The DOM threader will
> > > > > > > continue
> > > > > > >using the forward threader infrastructure while continuing to
> > > > > > > query
> > > > > > >DOM data structures, and only if the conditional does not
> > > > > > > relsolve,
> > > > > > >using the ranger. This gives us the best of both worlds, and
> > > > > > > is a
> > > > > > >proven approach.
> > > > > > >
> > > > > > >Furthermore, as frange and prange come live in the next cycle,
> > > > > > > we
> > > > > > >can move away from the forward threader altogether, and just
> > > > > > > add
> > > > > > >another backward threader. This will not only remove the last
> > > > > > > use
> > > > > > >of the forward threader, but will allow us to remove at least
> > > > > > > 1 or 2
> > > > > > >threader instances.
> > > > > > Excellent.
> > > > > >
> > > > > > >
> > > > > > > * Convert conditional folding to use the method used by the
> > > > > > > ranger and
> > > > > > >evrp. Previously DOM was calling into the guts of
> > > > > > >simplify_using_ranges::vrp_visit_cond_stmt. The blessed way
> > > > > > > now is
> > > > > > >using fold_cond() which rewrites the conditional and edges
> > > > > > >automatically.
> > > > > > >
> > > > > > >When legacy is removed, simplify_using_ranges will be further
> > > > > > >cleaned up, and there will only be one entry point into
> > > > > > > simplifying
> > > > > > >a statement.
> > > > > > Sure.
> > > > > >
> > > > > > >
> > > > > > > * DOM was setting global ranges determined from unreachable edges
> > > > > > > as a
> > > > > > >side-effect of using the evrp engine. We must handle these
> > > > > > > cases
> > > > > > >before nuking evrp, and DOM seems like a good fit. I've just
> > > > > > > moved
> > > > > > >the snippet to DOM, but it could live
Re: *PING* [PATCH 0/4] Use pointer arithmetic for array references [PR102043]
On Tue, Apr 26, 2022 at 04:40:33PM +0200, Hans-Peter Nilsson wrote: > > From: Thomas Koenig via Gcc-patches > > Date: Fri, 22 Apr 2022 15:59:45 +0200 > > > Hi Mikael, > > > > > Ping for the four patches starting at > > > https://gcc.gnu.org/pipermail/fortran/2022-April/057759.html : > > > https://gcc.gnu.org/pipermail/fortran/2022-April/057757.html > > > https://gcc.gnu.org/pipermail/fortran/2022-April/057760.html > > > https://gcc.gnu.org/pipermail/fortran/2022-April/057758.html > > > https://gcc.gnu.org/pipermail/fortran/2022-April/057761.html > > > > > > Richi accepted the general direction and the middle-end interaction. > > > I need a fortran frontend ack as well. > > > > Looks good to me. > > > > Thanks a lot for taking this on! This would have been a serious > > regression if released with gcc 12. > > > > Best regards > > > > Thomas > > These, or specifically r12-8227-g89ca0fffa48b79, "fortran: > Pre-evaluate string pointers. [PR102043]" have further > exposed (the issue existed before but now fails for more > platforms) PR78054 "gfortran.dg/pr70673.f90 FAILs at -O0", > at least for cris-elf and apparently also > s390x-ibm-linux-gnu. > > In the PR it is mentioned that running the test through > valgrind shows invalid accesses also on x86_64-linux-gnu. > Could it be that the test-case is invalid and has undefined > behavior? I don't know fortran so I can't tell. > > That exact commit causing a regression for s390x is somewhat > an assumption based on posted date and testresults, as the > s390x results don't include a git version. (@Stefansf: I'm > referring to > https://gcc.gnu.org/pipermail/gcc-testresults/2022-April/760060.html > https://gcc.gnu.org/pipermail/gcc-testresults/2022-April/760137.html > Perhaps that tester isn't using the contrib/gcc_update and > contrib/test_summary scripts, thus no LAST_UPDATED > included?) Indeed the reports don't include a git commit id. We are using both scripts. However, since the git repository is setup differently in our case, we had been using `gcc_update --touch` only. Thus the files LAST_UPDATED as well as gcc/REVISION were not created. I changed that such that both are created, now. Thanks for letting me know! Cheers, Stefan
Re: [COMMITTED] Denormalize VR_VARYING to VR_RANGE before passing it to set_range_info_raw.
On Mon, May 2, 2022 at 8:34 AM Richard Biener wrote: > > On Sun, May 1, 2022 at 2:15 PM Aldy Hernandez via Gcc-patches > wrote: > > > > We are ICEing in set_range_info_raw because value_range_kind cannot be > > VR_VARYING, since SSA_NAME_RANGE_TYPE can only hold VR_RANGE / > > VR_ANTI_RANGE. Most of the time setting a VR_VARYING as a global > > range makes no sense. However, we can have a range spanning the > > entire domain (VR_RANGE of [MIN,MAX] which is essentially a > > VR_VARYING), if the nonzero bits are set. > > > > This was working before because set_range_info_raw allows setting > > VR_RANGE of [MIN, MAX]. However, when going through an irange, we > > normalize this to a VR_VARYING, thus causing the ICE. It's > > interesting that other calls to set_range_info with an irange haven't > > triggered this. > > > > One solution would be to just ignore VR_VARYING and bail, since > > set_range_info* is really an update of the current range semantic > > wise. After all, we keep the nonzero bits which provide additional > > info. But this would be a change in behavior, so not suitable until > > after GCC 12 is released. So in order to keep with current behavior > > we can just denormalize the varying to VR_RANGE. > > But there's no point in storing such info - shouldn't the function simply > clear the range and return? Or at least avoid allocating a new range > -info and just return if none is associated with the SSA name at the moment? Yup. That's what the code block immediately following will do: clear the range (and free the storage), but only if the nonzero bits are also clear. This duality of ranges with nonzero bits and ranges is a bit annoying. I have code overhauling this for when we provide a way to store global (multi) ranges later this cycle. Nonzero bits will live in the range itself, and updating the global range / nonzero bits will have one entry point that takes a range. Also, nonzero bits and ranges will be kept up to date wrt each other automatically. Aldy
Re: [COMMITTED] Denormalize VR_VARYING to VR_RANGE before passing it to set_range_info_raw.
On Sun, May 1, 2022 at 2:15 PM Aldy Hernandez via Gcc-patches
wrote:
>
> We are ICEing in set_range_info_raw because value_range_kind cannot be
> VR_VARYING, since SSA_NAME_RANGE_TYPE can only hold VR_RANGE /
> VR_ANTI_RANGE. Most of the time setting a VR_VARYING as a global
> range makes no sense. However, we can have a range spanning the
> entire domain (VR_RANGE of [MIN,MAX] which is essentially a
> VR_VARYING), if the nonzero bits are set.
>
> This was working before because set_range_info_raw allows setting
> VR_RANGE of [MIN, MAX]. However, when going through an irange, we
> normalize this to a VR_VARYING, thus causing the ICE. It's
> interesting that other calls to set_range_info with an irange haven't
> triggered this.
>
> One solution would be to just ignore VR_VARYING and bail, since
> set_range_info* is really an update of the current range semantic
> wise. After all, we keep the nonzero bits which provide additional
> info. But this would be a change in behavior, so not suitable until
> after GCC 12 is released. So in order to keep with current behavior
> we can just denormalize the varying to VR_RANGE.
But there's no point in storing such info - shouldn't the function simply
clear the range and return? Or at least avoid allocating a new range
-info and just return if none is associated with the SSA name at the moment?
> Tested on x86-64 Linux.
>
> PR tree-optimization/105432
>
> gcc/ChangeLog:
>
> * tree-ssanames.cc (set_range_info): Denormalize VR_VARYING to
> VR_RANGE before passing a piecewise range to set_range_info_raw.
> ---
> gcc/tree-ssanames.cc | 11 ++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/tree-ssanames.cc b/gcc/tree-ssanames.cc
> index c957597af4f..05536cd2f74 100644
> --- a/gcc/tree-ssanames.cc
> +++ b/gcc/tree-ssanames.cc
> @@ -395,8 +395,17 @@ set_range_info (tree name, enum value_range_kind
> range_type,
> {
>gcc_assert (!POINTER_TYPE_P (TREE_TYPE (name)));
>
> - /* A range of the entire domain is really no range at all. */
>tree type = TREE_TYPE (name);
> + if (range_type == VR_VARYING)
> +{
> + /* SSA_NAME_RANGE_TYPE can only hold a VR_RANGE or
> +VR_ANTI_RANGE. Denormalize VR_VARYING to VR_RANGE. */
> + range_type = VR_RANGE;
> + gcc_checking_assert (min == wi::min_value (type));
> + gcc_checking_assert (max == wi::max_value (type));
> +}
> +
> + /* A range of the entire domain is really no range at all. */
>if (min == wi::min_value (TYPE_PRECISION (type), TYPE_SIGN (type))
>&& max == wi::max_value (TYPE_PRECISION (type), TYPE_SIGN (type)))
> {
> --
> 2.35.1
>
Re: [gcov v2 11/14] gcov: Record EOF error during read
> @@ -385,7 +395,11 @@ gcov_read_bytes (void *buffer, unsigned count)
>
>unsigned read = fread (buffer, count, 1, gcov_var.file);
>if (read != 1)
> -return NULL;
> +{
> + if (feof (gcov_var.file))
> + gcov_var.error = GCOV_FILE_EOF;
> + return NULL;
> +}
Hello.
This hunk is causing troubles when we instrument a binary and it's the first
time we dump to it.
See:
gcc --coverage main.c && ./a.out
libgcov profiling error:/home/marxin/Programming/testcases/a-main.gcda:Error
writing
Can you please fix it?
Thanks,
Martin
Re: [patch, fortran, doc] Mention new CONVERT options for POWER
On Sat, Apr 30, 2022 at 1:26 AM Bernhard Reutner-Fischer wrote: > > On Fri, 29 Apr 2022 20:03:55 +0200 > Thomas Koenig wrote: > > > On 28.04.22 19:17, Bernhard Reutner-Fischer wrote: > > > ISTM that this should be s/mod.e/mode./ ? > > > > Yep, fixed as obvious and simple on trunk with > > r13-49-g4a8b45e8bc8246bd141dad65f571a3e0cc499c6b . > > thanks! > > > > OK for backport to gcc-12? (This is both extremely safe and > > not particularly important :-) > > I'd say yes because it's 1) doc 2) trivial 3) obvious :) > > But formally, i think RM approval is needed ATM. > Richard? OK.
Re: [PATCH] Replace EVRP in DOM with ranger.
On Fri, Apr 29, 2022 at 6:22 PM Aldy Hernandez wrote:
>
> On Fri, Apr 29, 2022 at 12:13 PM Richard Biener
> wrote:
> >
> > On Fri, Apr 29, 2022 at 11:53 AM Aldy Hernandez wrote:
> > >
> > > On Fri, Apr 29, 2022 at 9:09 AM Richard Biener
> > > wrote:
> > > >
> > > > On Thu, Apr 28, 2022 at 8:44 PM Jeff Law via Gcc-patches
> > > > wrote:
> > > > >
> > > > >
> > > > >
> > > > > On 4/28/2022 10:30 AM, Aldy Hernandez wrote:
> > > > > > [Jeff, this is the same patch I sent you last week with minor tweaks
> > > > > > to the commit message.]
> > > > > And I just dropped it into the tester. We should have the vast
> > > > > majority
> > > > > of targets tested by this time tomorrow.
> > > > >
> > > > > >
> > > > > > [Despite the verbosity of the message, this is actually a pretty
> > > > > > straightforward patch. It should've gone in last cycle, but there
> > > > > > was a nagging regression I couldn't get to until after stage1
> > > > > > had closed.]
> > > > > You had other, non-work/gcc priorities. No worries at missing gcc-12.
> > > > >
> > > > >
> > > > > >
> > > > > > There are 3 uses of EVRP in DOM that must be converted.
> > > > > > Unfortunately, they need to be converted in one go, so further
> > > > > > splitting of this patch would be problematic.
> > > > > ACK
> > > > >
> > > > >
> > > > > > There's nothing here earth shattering. It's all pretty obvious in
> > > > > > retrospect, but I've added a short description of each use to aid in
> > > > > > reviewing:
> > > > > >
> > > > > > * Convert evrp use in cprop to ranger.
> > > > > >
> > > > > >This is easy, as cprop in DOM was converted to the ranger API
> > > > > > last
> > > > > >cycle, so this is just a matter of using a ranger instead of an
> > > > > >evrp_range_analyzer.
> > > > > Yea. We may have covered this before, but what does ranger do with a
> > > > > conditional equivalence?
> > > > >
> > > > > ie if we have something like
> > > > >
> > > > > if (a == b)
> > > > >{
> > > > > blah blah
> > > > >}
> > > > >
> > > > > Within the true arm we know there's an equivalence. *But* we don't
> > > > > want
> > > > > to just blindly replace a with b or vice-versa. The equivalence is
> > > > > primarily useful for simplification rather than propagation.
> > > > >
> > > > > In fact, we every much do not want to cprop in these cases. It
> > > > > rarely
> > > > > helps in a meaningful way and there's no good way to know which is the
> > > > > better value to use. Canonicalization on SSA_NAMEs doesn't really
> > > > > help
> > > > > due to SSA_NAME recycling.
> > > > >
> > > > >
> > > > > >
> > > > > > * Convert evrp use in threader to ranger.
> > > > > >
> > > > > >The idea here is to use the hybrid approach we used for the
> > > > > > initial
> > > > > >VRP threader conversion last cycle. The DOM threader will
> > > > > > continue
> > > > > >using the forward threader infrastructure while continuing to
> > > > > > query
> > > > > >DOM data structures, and only if the conditional does not
> > > > > > relsolve,
> > > > > >using the ranger. This gives us the best of both worlds, and is
> > > > > > a
> > > > > >proven approach.
> > > > > >
> > > > > >Furthermore, as frange and prange come live in the next cycle, we
> > > > > >can move away from the forward threader altogether, and just add
> > > > > >another backward threader. This will not only remove the last
> > > > > > use
> > > > > >of the forward threader, but will allow us to remove at least 1
> > > > > > or 2
> > > > > >threader instances.
> > > > > Excellent.
> > > > >
> > > > > >
> > > > > > * Convert conditional folding to use the method used by the ranger
> > > > > > and
> > > > > >evrp. Previously DOM was calling into the guts of
> > > > > >simplify_using_ranges::vrp_visit_cond_stmt. The blessed way now
> > > > > > is
> > > > > >using fold_cond() which rewrites the conditional and edges
> > > > > >automatically.
> > > > > >
> > > > > >When legacy is removed, simplify_using_ranges will be further
> > > > > >cleaned up, and there will only be one entry point into
> > > > > > simplifying
> > > > > >a statement.
> > > > > Sure.
> > > > >
> > > > > >
> > > > > > * DOM was setting global ranges determined from unreachable edges
> > > > > > as a
> > > > > >side-effect of using the evrp engine. We must handle these cases
> > > > > >before nuking evrp, and DOM seems like a good fit. I've just
> > > > > > moved
> > > > > >the snippet to DOM, but it could live anywhere else we do a DOM
> > > > > >walk.
> > > > > It was a semi-desirable to record/refine global ranges in DOM, but
> > > > > I'd
> > > > > be surprised if too much code depended on that. Presumably there's a
> > > > > testcase in the suite which we don't handle well if we don't let DOM
> > > > > refine/record global ranges?
> > > > >
> > > > > >
> > > > > >For the record, this is the
