[RFC] genoutput: Error on unresolved iterator
I just ran into an unresolved iterator
https://gcc.gnu.org/pipermail/gcc-patches/2024-July/657360.html
which motivated me to dig into genoutput.cc where in process_template()
we already emit an error but only if the new compact syntax is used.
There is probably a reason for limiting the check to the new compact
syntax. However, after implementing my own check I realized that both
are basically the same. I also did a quick build while removing the
limitation to the new compact syntax on x86_64, aarch64, powerpc64le,
s390 and it all went through. Skimming through target MD files I also
couldn't find a reason why a test for angle brackets shouldn't be done
for non-compact syntax but maybe I'm just missing something.
Long story short: would it be fine to perform an unresolved iterator check for
non-compact syntax or would that break any target?
---
gcc/genoutput.cc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gcc/genoutput.cc b/gcc/genoutput.cc
index efd81766bb5..53a0d0790ae 100644
--- a/gcc/genoutput.cc
+++ b/gcc/genoutput.cc
@@ -702,7 +702,7 @@ process_template (class data *d, const char *template_code)
message_at (d->loc, "trailing whitespace in output template");
/* Check for any unexpanded iterators. */
- if (bp[0] != '*' && d->compact_syntax_p)
+ if (bp[0] != '*')
{
const char *p = cp;
const char *last_bracket = nullptr;
--
2.45.2
Re: Insn combine trying (ior:HI (clobber:HI (const_int 0)))
Am 15.07.24 um 19:53 schrieb Richard Sandiford:
Georg-Johann Lay writes:
In a test case I see insn combine trying to match such
expressions, which do not make any sense to me, like:
Trying 2 -> 7:
2: r45:HI=r48:HI
REG_DEAD r48:HI
7: {r47:HI=r45:HI|r46:PSI#0;clobber scratch;}
REG_DEAD r46:PSI
REG_DEAD r45:HI
Failed to match this instruction:
(parallel [
(set (reg:HI 47 [ _4 ])
(ior:HI (clobber:HI (const_int 0 [0]))
(reg:HI 48)))
(clobber (scratch:QI))
])
and many other occasions like that.
Is this just insn combine doing its business?
Or should this be some sensible RTL instead?
Seen on target avr with v14 and trunk,
attached test case and dump compiled with
(clobber:M (const_int 0)) is combine's way of representing
"something went wrong here". And yeah, recog acts as an error
detection mechanism in these cases. In other words, the idea
is that recog should eventually fail on nonsense rtl like that,
so earlier code doesn't need to check explicitly.
Richard
Hi Richard,
I never saw this before; and the point is that in some functions,
combine is almost exclusively producing these rtxes with clobber
const_int, but almost no meaningful combinations. Like in:
typedef __UINT32_TYPE__ u32;
typedef __uint24 u24;
u24 ior24_32_8 (u24 a, u32 b)
{
return a | (u24) b << 8;
}
u24 ior24_24_8 (u24 a, u24 b)
{
return a | (u24) b << 8;
}
In ior24_24_8() all goes fine: combine tries sensible combinations,
and uses them (provided the backend has patterns for it of course).
With ior24_32_8() however, it is almost only silly combinations
(or "something goes wrong in combine"), even though the assembly
code for either functions could be exactly the same (the only
difference is that hard reg R21 is input to the 1st function).
As an example, for the 2nd function it tries
(set (reg:PSI 47 [ _4 ])
(ior:PSI (ashift:PSI (reg:PSI 50)
(const_int 8 [0x8]))
(reg:PSI 49)))
but for the 1st function it reads:
(set (reg:PSI 48 [ _5 ])
(ior:PSI (ashift:PSI (clobber:PSI (const_int 0 [0]))
(const_int 8 [0x8]))
(reg:PSI 50)))
Johann
Re: insn attributes: Support blocks of C-code?
Am 13.07.24 um 13:44 schrieb Richard Sandiford:
Georg-Johann Lay writes:
So I had that situation where in an insn attribute, providing
a block of code (rather than just an expression) would be
useful.
Expressions can provided by means of symbol_ref, like in
(set (attr "length")
(symbol_ref ("1 + GET_MODE_SIZE (mode)")))
However providing a block of code gives a syntax error from
the compiler, *NOT* from md_reader:
(set (attr "length")
(symbol_ref
{
int len = 1;
return len;
}))
This means such syntax is already supported to some degree,
there's just no semantics assigned to such code.
Blocks of code are already supported in insn predicates,
like in
(define_predicate "my_operand"
(match_code "code_label,label_ref,symbol_ref,plus,const")
{
some code...
return true-or-false;
})
In the insn attribute case, I hacked a bit and supported
blocks of code like in the example above. The biggest change
is that class attr_desc has to be moved from genattrtab.cc to
read-md.h so that it is a complete type as required by
md_reader::fprint_c_condition().
That method prints to code for symbol_ref and some others, and
it has to know the type of the attribute, like "int" for the
"length" attribute. The implementation in fprint_c_condition()
is straight forward:
When cond (which is the payload string of symbol_ref, including the
'{}'s) starts with '{', the print a lambda that's called in place,
like in
print "( [&]() -> () )"
The "&" capture is required so that variables like "insn" are
accessible. "operands[]" and "which_alternative" are global,
thus also accessible.
Attached is the code I have so far (which is by no means a
proposed patch, so I am posting here on gcc@).
As far as I can tell, there is no performance penalty, e.g.
in build times, when the feature is not used. Of course instead
of such syntax, a custom function could be used, or the
braces-brackets-parentheses-gibberish could be written out
in the symbol_ref as an expression. Though I think this
could be a nice addition, in particular because the scanning
side in md_reader already supports the syntax.
Looks good to me. I know you said it wasn't a patch submission,
but it looks mostly ready to go. Some comments below:
diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
index 7f4335e0aac..3e46693e8c2 100644
--- a/gcc/doc/md.texi
+++ b/gcc/doc/md.texi
@@ -10265,6 +10265,56 @@ so there is no need to explicitly convert the
expression into a boolean
(match_test "(x & 2) != 0")
@end smallexample
+@cindex @code{symbol_ref} and attributes
+@item (symbol_ref "@var{quoted-c-expr}")
+
+Specifies the value of the attribute sub-expression as a C expression,
+where the surrounding quotes are not part of the expression.
+Similar to @code{match_test}, variables @var{insn}, @var{operands[]}
+and @var{which_alternative} are available. Moreover, code and mode
+attributes can be used to compose the resulting C expression, like in
+
+@smallexample
+(set (attr "length")
+ (symbol_ref ("1 + GET_MODE_SIZE (mode)")))
+@end smallexample
+
+where the according insn has exactly one mode iterator.
+See @ref{Mode Iterators} and @ref{Code Iterators}.
I got the impression s/See @ref/@xref/ was recommended for sentence
references.
+
+@item (symbol_ref "@{ @var{quoted-c-code} @}")
+@itemx (symbol_ref @{ @var{c-code} @})
+
+The value of this subexpression is determined by running a block
+of C code which returns the desired value.
+The braces are part of the code, whereas the quotes in the quoted form are not.
+
+This variant of @code{symbol_ref} allows for more comlpex code than
+just a single C expression, like for example:
+
+@smallexample
+(set (attr "length")
+ (symbol_ref
+ @{
+int len;
+some_function (insn, , mode, & len);
+return len;
+ @}))
+@end smallexample
+
+for an insn that has one code iterator and one mode iterator.
+Again, variables @var{insn}, @var{operands[]} and @var{which_alternative}
+can be used. The unquoted form only supports a subset of C,
+for example no C comments are supported, and strings that contain
+characters like @samp{@}} are problematic and may need to be escaped
+as @samp{\@}}.
By unquoted form, do you mean (symbol_ref { ... })? I'd have expected
that to be better than "{ ... }" (or at least, I thought that was the
intention when { ... } was added). I was going to suggest not documenting
the "{ ... }" form until I saw this.
+
+The return type is @code{int} for the @var{length} attribute, and
+@code{enum attr_@var{name}} for an insn attribute named @var{name}.
+The types and available enum values can be looked up in
+@file{$builddir/gcc/insn-attr-common.h}.
+
+
@cindex @code{le} and attributes
@cindex @code{leu} and attributes
@cindex @code{lt} and attributes
diff --git a/gcc/genattrtab.cc b/gcc/genattrtab.cc
index 03c7d6c74a3..20d45ba88af 100644
--- a/gcc/genattrtab.cc
+++ b/gcc/genattrtab.cc
@@ -168,22
Re: How to implement Native TLS for a specific platform?
Hi Julian, You can check how we did it for ARC. In a nutshell, you need to define HAVS_AS_TLS macro, you need to legitimize the new TLS address and calls. Please have a look in arc.cc and search for TLS, also use git blame to see the original patches. Of course, there are different ways to implement TLS, in ARC is the simplest solution. Also, u need to hack the assembler, linker and the OS for a full implementation. Cheers, Claudiu On Tue, Jul 9, 2024 at 7:14 PM Julian Waters via Gcc wrote: > > Hi all, > > I'm currently trying to implement Native TLS on a platform that gcc uses > emutls for at the moment, but I can't seem to figure out where and how to > implement it. I have a rough idea of the assembly required for TLS on this > platform, but I don't know where to plug it in to the compiler to make it > work. Could someone point me in the right direction for implementing TLS > for a platform that doesn't have it implemented at the moment? > > I'm aware that I am being vague as to which platform I want to implement it > for. It's a platform that is likely low priority in the eyes of most gcc > maintainers, so I'm deliberately avoiding mentioning what platform it is so > I don't get crickets for a reply :) > > best regards, > Julian
Re: Insn combine trying (ior:HI (clobber:HI (const_int 0)))
On 7/16/24 4:29 AM, Georg-Johann Lay wrote:
Am 15.07.24 um 19:53 schrieb Richard Sandiford:
Georg-Johann Lay writes:
In a test case I see insn combine trying to match such
expressions, which do not make any sense to me, like:
Trying 2 -> 7:
2: r45:HI=r48:HI
REG_DEAD r48:HI
7: {r47:HI=r45:HI|r46:PSI#0;clobber scratch;}
REG_DEAD r46:PSI
REG_DEAD r45:HI
Failed to match this instruction:
(parallel [
(set (reg:HI 47 [ _4 ])
(ior:HI (clobber:HI (const_int 0 [0]))
(reg:HI 48)))
(clobber (scratch:QI))
])
and many other occasions like that.
Is this just insn combine doing its business?
Or should this be some sensible RTL instead?
Seen on target avr with v14 and trunk,
attached test case and dump compiled with
(clobber:M (const_int 0)) is combine's way of representing
"something went wrong here". And yeah, recog acts as an error
detection mechanism in these cases. In other words, the idea
is that recog should eventually fail on nonsense rtl like that,
so earlier code doesn't need to check explicitly.
Richard
Hi Richard,
I never saw this before; and the point is that in some functions,
combine is almost exclusively producing these rtxes with clobber
const_int, but almost no meaningful combinations. Like in:
Perhaps, but the convention of using a (clobber (const_int 0)) for cases
where combine gets "lost" has been around for decades.
Jeff
Safe Cascading Frees
Document number: SCF4C++00
Date: 2024-7-16
Audience: GCC email list
Reply-to: marco.devill...@gmail.com, gcc@gcc.gnu.org
I. Introduction
Because C++ smart pointers are based on RAII it is easy to trigger an
overflow of the C stack since destructors call each other. Smart
pointers are supposed to be safe, smart pointers are likely to be used
extensively in the future, and this behaviour could make a large
number of C++ programs core dump unexpectedly.
This proposal is to remove this behaviour from GCCs standard library
and also showcases a small trick by which that can be done.
II. Motivation and Scope
We all want smart pointers since they allow for easy and safe memory
management, this desire is only expected to increase over the
following decades.
However due to RAII semantics it's easy to trigger an overflow of the
C stack once garbage goes out of scope. Observe the following trivial
program:
#include
#include
struct list_node {
using ptr = std::unique_ptr;
~list_node() {
}
int x;
ptr next;
};
int main() {
list_node::ptr next = nullptr;
for(int i = 0; i < 10; ++i) { // decrease value to see it not segfault
next = list_node::ptr(new list_node{i, std::move(next)});
}
}
Cascading frees will make this program core dump depending the size of
the list. Please note, that that program will segfault on for current
data-driven days relatively tiny sizes.
I give it to you that this is unsafe and unwanted behaviour since now
every program employing nested structures can core dump easily and
unexpectedly. For example: GUIs, editors, parsers, transformers, etc.
The problem is only expected to worsen with more developers using safe
pointers.
The proposal is to remove this behaviour from the GCC standard library
by hardening smart pointers in the following manner: instead of
calling garbage recursively garbage is first offloaded to a stack and
that stack destroys objects until it is empty. Or by any other means
that removes segfaulting cascading frees.
A reference implementation (not concurrent) for unique pointers is
given as an Addendum. (The code is due to Alipha on libera.net).
IV. Impact On the Standard
This shouldn't impact other parts of the standard.
V. Design Decisions
Offload destructor calls to an explicit stack to make these calls
sequential instead of recursive. A non-concurrent implementation of
unique pointers is given below.
VI. Technical Specifications
This shouldn't change anything.
VII. Acknowledgements
This cascading free behaviour was noticed during the development of
the Egel language interpreter, and the author has great interest in
having this resolved. The problem was discussed on various channels
and together with Alipha on libera.net a solution was developed.
VIII. Addendum
#include
#include
#include
#include
#include
namespace detail {
inline bool doing_cleanup = false;
inline std::vector> ptr_cleanup;
}
template
class safe_unique_ptr {
public:
safe_unique_ptr() : ptr() {}
safe_unique_ptr(T *p) : ptr(p) {}
safe_unique_ptr(safe_unique_ptr &&other) :
ptr(std::exchange(other.ptr, nullptr)) {}
safe_unique_ptr &operator=(safe_unique_ptr &&other) {
cleanup(ptr);
ptr = std::exchange(other.ptr, nullptr);
return *this;
}
T &operator*() const { return *ptr; }
T *operator->() const { return ptr; }
~safe_unique_ptr() { cleanup(ptr); }
private:
void cleanup(T *p) {
using namespace detail;
if(!p) return;
if(!doing_cleanup) {
doing_cleanup = true;
delete p;
while(!ptr_cleanup.empty()) {
std::function deleter = ptr_cleanup.back();
ptr_cleanup.pop_back();
deleted();
}
doing_cleanup = false;
} else {
ptr_cleanup.push_back([p]() { delete p; });
}
}
T *ptr;
};
struct list_node {
using ptr = safe_unique_ptr;
~list_node() {}
int x;
ptr next;
};
Re: Safe Cascading Frees
On Tue, 16 Jul 2024, 18:51 M.C.A. (Marco) Devillers via Gcc, <
gcc@gcc.gnu.org> wrote:
> Document number: SCF4C++00
> Date: 2024-7-16
> Audience: GCC email list
> Reply-to: marco.devill...@gmail.com, gcc@gcc.gnu.org
>
> I. Introduction
>
> Because C++ smart pointers are based on RAII it is easy to trigger an
> overflow of the C stack since destructors call each other. Smart
> pointers are supposed to be safe, smart pointers are likely to be used
> extensively in the future, and this behaviour could make a large
> number of C++ programs core dump unexpectedly.
> This proposal is to remove this behaviour from GCCs standard library
>
Where does it exist in the library?
The problem in your program is easily avoided, without changing the
standard or GCC's implementation of the standard library.
and also showcases a small trick by which that can be done.
>
> II. Motivation and Scope
>
> We all want smart pointers since they allow for easy and safe memory
> management, this desire is only expected to increase over the
> following decades.
>
> However due to RAII semantics it's easy to trigger an overflow of the
> C stack once garbage goes out of scope. Observe the following trivial
> program:
>
> #include
> #include
>
> struct list_node {
>using ptr = std::unique_ptr;
>~list_node() {
>}
>
>int x;
>ptr next;
> };
>
> int main() {
>list_node::ptr next = nullptr;
>
>for(int i = 0; i < 10; ++i) { // decrease value to see it not
> segfault
>next = list_node::ptr(new list_node{i, std::move(next)});
>}
> }
>
> Cascading frees will make this program core dump depending the size of
> the list. Please note, that that program will segfault on for current
> data-driven days relatively tiny sizes.
>
So don't do that then. It's easy to add a loop to clean up the list. Or
create an actual list class to manage the nodes and do that loop in its
destructor. Nobody is forced to define a list only in terms of nodes,
rather than a list class that manages the nodes.
> I give it to you that this is unsafe and unwanted behaviour since now
> every program employing nested structures can core dump easily and
> unexpectedly. For example: GUIs, editors, parsers, transformers, etc.
> The problem is only expected to worsen with more developers using safe
> pointers.
>
> The proposal is to remove this behaviour from the GCC standard library
> by hardening smart pointers in the following manner: instead of
> calling garbage recursively garbage is first offloaded to a stack and
> that stack destroys objects until it is empty. Or by any other means
> that removes segfaulting cascading frees.
>
> A reference implementation (not concurrent) for unique pointers is
> given as an Addendum. (The code is due to Alipha on libera.net).
>
> IV. Impact On the Standard
>
> This shouldn't impact other parts of the standard.
>
> V. Design Decisions
>
> Offload destructor calls to an explicit stack to make these calls
> sequential instead of recursive. A non-concurrent implementation of
> unique pointers is given below.
>
> VI. Technical Specifications
>
> This shouldn't change anything.
>
> VII. Acknowledgements
>
> This cascading free behaviour was noticed during the development of
> the Egel language interpreter, and the author has great interest in
> having this resolved. The problem was discussed on various channels
> and together with Alipha on libera.net a solution was developed.
>
> VIII. Addendum
>
> #include
> #include
> #include
> #include
> #include
>
> namespace detail {
> inline bool doing_cleanup = false;
> inline std::vector> ptr_cleanup;
> }
>
> template
> class safe_unique_ptr {
> public:
> safe_unique_ptr() : ptr() {}
> safe_unique_ptr(T *p) : ptr(p) {}
>
> safe_unique_ptr(safe_unique_ptr &&other) :
> ptr(std::exchange(other.ptr, nullptr)) {}
>
> safe_unique_ptr &operator=(safe_unique_ptr &&other) {
> cleanup(ptr);
> ptr = std::exchange(other.ptr, nullptr);
> return *this;
> }
>
> T &operator*() const { return *ptr; }
> T *operator->() const { return ptr; }
>
> ~safe_unique_ptr() { cleanup(ptr); }
>
> private:
> void cleanup(T *p) {
> using namespace detail;
>
> if(!p) return;
>
> if(!doing_cleanup) {
> doing_cleanup = true;
> delete p;
>
> while(!ptr_cleanup.empty()) {
> std::function deleter = ptr_cleanup.back();
> ptr_cleanup.pop_back();
> deleted();
> }
>
> doing_cleanup = false;
> } else {
> ptr_cleanup.push_back([p]() { delete p; });
> }
> }
>
> T *ptr;
> };
>
> struct list_node {
> using ptr = safe_unique_ptr;
> ~list_node() {}
>
> int x;
> ptr next;
> };
>
Re: Safe Cascading Frees
On Tue, 16 Jul 2024 at 19:12, Jonathan Wakely wrote:
>
>
>
> On Tue, 16 Jul 2024, 18:51 M.C.A. (Marco) Devillers via Gcc,
> wrote:
>>
>> Document number: SCF4C++00
>> Date: 2024-7-16
>> Audience: GCC email list
>> Reply-to: marco.devill...@gmail.com, gcc@gcc.gnu.org
>>
>> I. Introduction
>>
>> Because C++ smart pointers are based on RAII it is easy to trigger an
>> overflow of the C stack since destructors call each other. Smart
>> pointers are supposed to be safe, smart pointers are likely to be used
>> extensively in the future, and this behaviour could make a large
>> number of C++ programs core dump unexpectedly.
>> This proposal is to remove this behaviour from GCCs standard library
>
>
> Where does it exist in the library?
>
> The problem in your program is easily avoided, without changing the standard
> or GCC's implementation of the standard library.
>
>> and also showcases a small trick by which that can be done.
>>
>> II. Motivation and Scope
>>
>> We all want smart pointers since they allow for easy and safe memory
>> management, this desire is only expected to increase over the
>> following decades.
>>
>> However due to RAII semantics it's easy to trigger an overflow of the
>> C stack once garbage goes out of scope. Observe the following trivial
>> program:
>>
>> #include
>> #include
>>
>> struct list_node {
>>using ptr = std::unique_ptr;
>>~list_node() {
>>}
>>
>>int x;
>>ptr next;
>> };
>>
>> int main() {
>>list_node::ptr next = nullptr;
>>
>>for(int i = 0; i < 10; ++i) { // decrease value to see it not segfault
>>next = list_node::ptr(new list_node{i, std::move(next)});
>>}
>> }
>>
>> Cascading frees will make this program core dump depending the size of
>> the list. Please note, that that program will segfault on for current
>> data-driven days relatively tiny sizes.
>
>
> So don't do that then. It's easy to add a loop to clean up the list. Or
> create an actual list class to manage the nodes and do that loop in its
> destructor. Nobody is forced to define a list only in terms of nodes, rather
> than a list class that manages the nodes.
Or you can put the loop in the node destructor:
~list_node() {
while (next)
next = std::move(next->next);
}
Unlike your cleanup stack, this doesn't require any additional memory
allocation or synchronization.
Re: Safe Cascading Frees
All your proposals now boil down to: Do explicit memory management
whereas the developer supposes that is handled for them.
On Tue, Jul 16, 2024 at 8:24 PM Jonathan Wakely wrote:
>
> On Tue, 16 Jul 2024 at 19:12, Jonathan Wakely wrote:
> >
> >
> >
> > On Tue, 16 Jul 2024, 18:51 M.C.A. (Marco) Devillers via Gcc,
> > wrote:
> >>
> >> Document number: SCF4C++00
> >> Date: 2024-7-16
> >> Audience: GCC email list
> >> Reply-to: marco.devill...@gmail.com, gcc@gcc.gnu.org
> >>
> >> I. Introduction
> >>
> >> Because C++ smart pointers are based on RAII it is easy to trigger an
> >> overflow of the C stack since destructors call each other. Smart
> >> pointers are supposed to be safe, smart pointers are likely to be used
> >> extensively in the future, and this behaviour could make a large
> >> number of C++ programs core dump unexpectedly.
> >> This proposal is to remove this behaviour from GCCs standard library
> >
> >
> > Where does it exist in the library?
> >
> > The problem in your program is easily avoided, without changing the
> > standard or GCC's implementation of the standard library.
> >
> >> and also showcases a small trick by which that can be done.
> >>
> >> II. Motivation and Scope
> >>
> >> We all want smart pointers since they allow for easy and safe memory
> >> management, this desire is only expected to increase over the
> >> following decades.
> >>
> >> However due to RAII semantics it's easy to trigger an overflow of the
> >> C stack once garbage goes out of scope. Observe the following trivial
> >> program:
> >>
> >> #include
> >> #include
> >>
> >> struct list_node {
> >>using ptr = std::unique_ptr;
> >>~list_node() {
> >>}
> >>
> >>int x;
> >>ptr next;
> >> };
> >>
> >> int main() {
> >>list_node::ptr next = nullptr;
> >>
> >>for(int i = 0; i < 10; ++i) { // decrease value to see it not
> >> segfault
> >>next = list_node::ptr(new list_node{i, std::move(next)});
> >>}
> >> }
> >>
> >> Cascading frees will make this program core dump depending the size of
> >> the list. Please note, that that program will segfault on for current
> >> data-driven days relatively tiny sizes.
> >
> >
> > So don't do that then. It's easy to add a loop to clean up the list. Or
> > create an actual list class to manage the nodes and do that loop in its
> > destructor. Nobody is forced to define a list only in terms of nodes,
> > rather than a list class that manages the nodes.
>
> Or you can put the loop in the node destructor:
>
>~list_node() {
> while (next)
>next = std::move(next->next);
>}
>
> Unlike your cleanup stack, this doesn't require any additional memory
> allocation or synchronization.
Re: Safe Cascading Frees
On Tue, 16 Jul 2024 at 19:26, M.C.A. (Marco) Devillers wrote: > > All your proposals now boil down to: Do explicit memory management > whereas the developer supposes that is handled for them. There's no explicit memory management in the loop I showed. It's explicit control of lifetime, which is what all the containers in the C++ library do for you. If that's too much trouble to avoid unbounded stack growth, then use data structures somebody else has already written which work correctly.
