Re: insn attributes: Support blocks of C-code?

2024-07-13 Thread Richard Sandiford via Gcc
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

Re: Sourceware mitigating and preventing the next xz-backdoor

2024-07-13 Thread Trenton Davison via Gcc


Question about enabling additional flags in Ofast

2024-07-13 Thread Hanke Zhang via Gcc
Hi,

I'm attempting to enable more flags in Ofast, but I've encountered some issues.

For instance, if I want to add -flto-partition=one to Ofast, here is
the modification I made to opts.cc

/* -Ofast adds optimizations to -O3.  */
{ OPT_LEVELS_FAST, OPT_ffast_math, NULL, 1 },
{ OPT_LEVELS_FAST, OPT_fallow_store_data_races, NULL, 1 },
{ OPT_LEVELS_FAST, OPT_fsemantic_interposition, NULL, 0 },
++ { OPT_LEVELS_FAST, OPT_flto_partition_, NULL, LTO_PARTITION_ONE },

However, when I run the tests (make -k check), I encounter some
problems. One of the failed test files is gcc.dg/pr89342.c:

/* PR other/89342 */
/* { dg-do compile } */
/* { dg-options "-O0" } */

__attribute__((optimize("Ofast")))
void foo (void)
{
  __attribute__((optimize("no-inline")))
  void bar (void) {}
  bar ();
}

The error message is as follows:

../gcc/testsuite/gcc.dg/pr89342.c:7:1: internal compiler error:
‘global_options’ are modified in local context
7 | {
  | ^
0x9e8cf4 handle_optimize_attribute
../../gcc/c-family/c-attribs.cc:5568
0x8e0902 decl_attributes(tree_node**, tree_node*, int, tree_node*)
../../gcc/attribs.cc:872
0x8fe39c start_function(c_declspecs*, c_declarator*, tree_node*)
../../gcc/c/c-decl.cc:9519
0x962336 c_parser_declaration_or_fndef
../../gcc/c/c-parser.cc:2445
0x96b803 c_parser_external_declaration
../../gcc/c/c-parser.cc:1779
0x96c263 c_parser_translation_unit
../../gcc/c/c-parser.cc:1652
0x96c263 c_parse_file()
../../gcc/c/c-parser.cc:23372
0x9cf4a5 c_common_parse_file()
../../gcc/c-family/c-opts.cc:1240

Upon debugging, I found that the error occurs in the
cl_optimization_compare(gcc_options *ptr1, gcc_options *ptr2) function
in options-save.cc. Specifically, the discrepancy is here:

ptr1->x_flag_lto_partition: 2, ptr2->x_flag_lto_partition: 1

This discrepancy leads to the error. Could you advise on how to
resolve this issue?

(Note: My ultimate goal is not to add the -flto-partition=one flag.
This is just an example to illustrate the problem.)

Thank you for your assistance.

Thanks
Hanke Zhang


Re: IFNDR on UB? [was: Straw poll on shifts with out of range operands]

2024-07-13 Thread Martin Uecker via Gcc
Am Montag, dem 01.07.2024 um 15:19 +0200 schrieb Matthias Kretz:
> On Sunday, 30 June 2024 08:33:35 GMT+2 Martin Uecker wrote:
> > Am Sonntag, dem 30.06.2024 um 05:03 +0200 schrieb Matthias Kretz:
> > > On Saturday, 29 June 2024 16:20:55 GMT+2 Martin Uecker wrote:
> > > > Am Samstag, dem 29.06.2024 um 08:50 -0500 schrieb Matthias Kretz via 
> Gcc:
> > > > > 
,
..
> > 
> > > > But I am not sure how this is relevant here as this affects only
> > > > observable behavior and the only case where GCC does not seem to
> > > > already conform to this is volatile.
> > > 
> > > Now you lost me.
> > 
> > Consider the following example:
> > 
> > int f(int x)
> > {
> >  int r = 0;
> >  if (x < 10)
> >r = 1;
> >  if (x < 10)
> >__builtin_unreachable();
> >  return r;
> > }
> > 
> > But removing the store to 'r' here as GCC does:
> > 
> > https://godbolt.org/z/h7qqrGsbz
> > 
> > can simply be justified by the "as if" principle as
> > any other optimization, it does not need to rely on a weird
> > intepretation that the UB from __builin_unreachable() travels
> > back in time.
> 
> I don't know of anybody saying that "time-travel optimization" refers to 
> anything different than what you're showing here. 

The C++ standard allows also removing or changing previous
observable behavior.

"However, if any such execution contains an undefined operation,
this International Standard places no requirement on the implementation
executing that program with that input (not even with regard to
operations preceding the first undefined operation)."

This was discussed extensively in the past, e.g. when discussing
the specification for memset_explicit.


> The part that people find 
> scary is when this "as if" happens at a distance, like in
> https://godbolt.org/z/jP4x1c3E6

True, although without "true time travel" (i.e. excluding
time-travel affecting previous observable behavior), this is
limited in the damage it can do (e.g. can not undo changes
committed to a log or control inputs to an external machine)
and has a much simpler interpretation that does not need to
refer to metaphysical concepts: Instead of the UB  affecting 
previous behavior by travelling back in time, it just has
arbitrary behavior that might undo the store.

> 
> > > [...]
> > 
> > I think it is a good idea. The compiler can optionally treat UB as
> > a translation time error. We discussed similar ideas in the past
> > in WG14. But this will only work for very specific instances of UB
> > under certain conditions.
> 
> Yes. But it's an exact match of the "time-travel" cases. I.e. whenever const-
> prop determines "unreachable" the code could be ill-formed.

I am not sure it is so simple.  What about if all this is in
dead code?  I assume one needs a rule similar to "for a functions,
when it is not possible to invoke the function without reaching
an operation which has UB, then the program ill-formed." or
"if a function call expression always invokes UB when executed,
then...". I guess the later is would apply to the precondition
viplations you mentioned WG21 is considering.

> 
> 
> > > > Also IMHO this should be split up from
> > > > UBsan which has specific semantics and upstream dependencies
> > > > which are are not always ideal.  (But UBSan could share the
> > > > same infrastructure)
> > > 
> > > I'm not sure what you're thinking of here. UBsan detects UB at runtime
> > > whereas my '-fharden=1' proposal is about flagging UB as ill-formed on
> > > compile-time. So UBsan is a more verbose '-fharden=2' then?
> > 
> > Yes, I was talking about the -fharden=2 case. In principle UBSan
> > with traps instead of diagnostics would do this. In practice,
> > I think we need something which is not tied to UBSan.
> 
> Yes, basically a deployable variant of UBsan?
> 

Yes, there is something such as -fcf-protection
or -fvtable-verify

Martin

> 
> On Sunday, 30 June 2024 08:56:41 GMT+2 Martin Uecker wrote:
> > 0) nothing
> > 1) expands to __builtin_unreachable()
> > 2) expands to __builtin_trap()
> > 3) expands to a __builtin_warning (as suggested before
> > by Martin Sebor) that causes the backend to emit an error
> > in a very late pass when the __builtin_warning has not
> > been removed during optimization.
> 
> This __builtin_warning seems to be equivalent to my __error() function, using 
> a [[gnu::warning]] attribute instead of [[gnu::error]]. Which is certainly 
> another viable build/-fharden/whateverwecallit mode.
> 





Re: Question about enabling additional flags in Ofast

2024-07-13 Thread Andrew Pinski via Gcc
On Sat, Jul 13, 2024 at 9:38 AM Hanke Zhang via Gcc  wrote:
>
> Hi,
>
> I'm attempting to enable more flags in Ofast, but I've encountered some 
> issues.

I suspect you need to handle this in the driver specs instead of in
opts.cc. Since -flto-partition=one is more of a global flag rather
than an optimization flag.
common.opt does not mark -flto-partition= as an optimization flag either.
Plus IIRC flto-partition= needs to be passed down to the linker in
this case too.

Thanks,
Andrew Pinski

>
> For instance, if I want to add -flto-partition=one to Ofast, here is
> the modification I made to opts.cc
>
> /* -Ofast adds optimizations to -O3.  */
> { OPT_LEVELS_FAST, OPT_ffast_math, NULL, 1 },
> { OPT_LEVELS_FAST, OPT_fallow_store_data_races, NULL, 1 },
> { OPT_LEVELS_FAST, OPT_fsemantic_interposition, NULL, 0 },
> ++ { OPT_LEVELS_FAST, OPT_flto_partition_, NULL, LTO_PARTITION_ONE },
>
> However, when I run the tests (make -k check), I encounter some
> problems. One of the failed test files is gcc.dg/pr89342.c:
>
> /* PR other/89342 */
> /* { dg-do compile } */
> /* { dg-options "-O0" } */
>
> __attribute__((optimize("Ofast")))
> void foo (void)
> {
>   __attribute__((optimize("no-inline")))
>   void bar (void) {}
>   bar ();
> }
>
> The error message is as follows:
>
> ../gcc/testsuite/gcc.dg/pr89342.c:7:1: internal compiler error:
> ‘global_options’ are modified in local context
> 7 | {
>   | ^
> 0x9e8cf4 handle_optimize_attribute
> ../../gcc/c-family/c-attribs.cc:5568
> 0x8e0902 decl_attributes(tree_node**, tree_node*, int, tree_node*)
> ../../gcc/attribs.cc:872
> 0x8fe39c start_function(c_declspecs*, c_declarator*, tree_node*)
> ../../gcc/c/c-decl.cc:9519
> 0x962336 c_parser_declaration_or_fndef
> ../../gcc/c/c-parser.cc:2445
> 0x96b803 c_parser_external_declaration
> ../../gcc/c/c-parser.cc:1779
> 0x96c263 c_parser_translation_unit
> ../../gcc/c/c-parser.cc:1652
> 0x96c263 c_parse_file()
> ../../gcc/c/c-parser.cc:23372
> 0x9cf4a5 c_common_parse_file()
> ../../gcc/c-family/c-opts.cc:1240
>
> Upon debugging, I found that the error occurs in the
> cl_optimization_compare(gcc_options *ptr1, gcc_options *ptr2) function
> in options-save.cc. Specifically, the discrepancy is here:
>
> ptr1->x_flag_lto_partition: 2, ptr2->x_flag_lto_partition: 1
>
> This discrepancy leads to the error. Could you advise on how to
> resolve this issue?
>
> (Note: My ultimate goal is not to add the -flto-partition=one flag.
> This is just an example to illustrate the problem.)
>
> Thank you for your assistance.
>
> Thanks
> Hanke Zhang


gcc-14-20240713 is now available

2024-07-13 Thread GCC Administrator via Gcc
Snapshot gcc-14-20240713 is now available on
  https://gcc.gnu.org/pub/gcc/snapshots/14-20240713/
and on various mirrors, see https://gcc.gnu.org/mirrors.html for details.

This snapshot has been generated from the GCC 14 git branch
with the following options: git://gcc.gnu.org/git/gcc.git branch 
releases/gcc-14 revision 89f9342980b7976f98ba43fac6a64a7a2214b6e6

You'll find:

 gcc-14-20240713.tar.xz   Complete GCC

  SHA256=c874b176a442ecdcb04974c73ba12d6ab0e49f43f9fd4837ddd1c9fabdab426e
  SHA1=35356c79771021180e5f63ac0411c93b65023542

Diffs from 14-20240706 are available in the diffs/ subdirectory.

When a particular snapshot is ready for public consumption the LATEST-14
link is updated and a message is sent to the gcc list.  Please do not use
a snapshot before it has been announced that way.


[no subject]

2024-07-13 Thread Cliff Gresham via Gcc
Clifford Gresham