[PATCH] fortran: gfc_trans_subcomponent_assign fixes [PR113503]
Hi!
The r14-870 changes broke xtb package tests (reduced testcase is the first
one below) and caused ICEs on a test derived from that (the second one).
For the
x = T(u = trim (us(1)))
statement, before that change gfortran used to emit weird code with
2 trim calls:
_gfortran_string_trim (&len.2, (void * *) &pstr.1, 20, &us[0]);
if (len.2 > 0)
{
__builtin_free ((void *) pstr.1);
}
D.4275 = len.2;
t.0.u = (character(kind=1)[1:0] *) __builtin_malloc (MAX_EXPR <(sizetype)
D.4275, 1>);
t.0._u_length = D.4275;
_gfortran_string_trim (&len.4, (void * *) &pstr.3, 20, &us[0]);
(void) __builtin_memcpy ((void *) t.0.u, (void *) pstr.3, (unsigned long)
NON_LVALUE_EXPR );
if (len.4 > 0)
{
__builtin_free ((void *) pstr.3);
}
That worked at runtime, though it is wasteful.
That commit changed it to:
slen.3 = len.2;
t.0.u = (character(kind=1)[1:0] *) __builtin_malloc (MAX_EXPR <(sizetype)
slen.3, 1>);
t.0._u_length = slen.3;
_gfortran_string_trim (&len.2, (void * *) &pstr.1, 20, &us[0]);
(void) __builtin_memcpy ((void *) t.0.u, (void *) pstr.1, (unsigned long)
NON_LVALUE_EXPR );
if (len.2 > 0)
{
__builtin_free ((void *) pstr.1);
}
which results in -Wuninitialized warning later on and if one is unlucky and
the uninitialized len.2 variable is smaller than the trimmed length, it
results in heap overflow and often crashes later on.
The bug above is clear, len.2 is only initialized in the
_gfortran_string_trim (&len.2, (void * *) &pstr.1, 20, &us[0]);
call, but used before that. Now, the
slen.3 = len.2;
t.0.u = (character(kind=1)[1:0] *) __builtin_malloc (MAX_EXPR <(sizetype)
slen.3, 1>);
t.0._u_length = slen.3;
statements come from the alloc_scalar_allocatable_subcomponent call,
while
_gfortran_string_trim (&len.2, (void * *) &pstr.1, 20, &us[0]);
from the gfc_conv_expr (&se, expr); call which is done before the
alloc_scalar_allocatable_subcomponent call, but is only appended later on
with gfc_add_block_to_block (&block, &se.pre);
Now, obviously the alloc_scalar_allocatable_subcomponent emitted statements
can depend on the se.pre sequence statements which can compute variables
used by alloc_scalar_allocatable_subcomponent like the length.
On the other side, I think the se.pre sequence really shouldn't depend
on the changes done by alloc_scalar_allocatable_subcomponent, that is
initializing the FIELD_DECLs of the destination allocatable subcomponent
only, the gfc_conv_expr statements are already created, so all they could
in theory depend above is on t.0.u or t.0._u_length, but I believe if the
rhs dependened on the lhs content (which is allocated by those statements
but really uninitialized), it would need to be discovered by the dependency
analysis and forced into a temporary.
So, in order to fix the first testcase, the second hunk of the patch just
emits the se.pre block before the alloc_scalar_allocatable_subcomponent
changes rather than after it.
The second problem is an ICE on the second testcase. expr in the caller
(expr2 inside of alloc_scalar_allocatable_subcomponent) has
expr2->ts.u.cl->backend_decl already set, INTEGER_CST 20, but
alloc_scalar_allocatable_subcomponent overwrites it to a new VAR_DECL
which it assigns a value to before the malloc. That can work if the only
places the expr2->ts is ever used are in the same local block or its
subblocks (and only if it is dominated by the code emitted by
alloc_scalar_allocatable_subcomponent, so e.g. not if that call is inside
of a conditional code and use later unconditional), but doesn't work
if expr2->ts is used before that block or after it. So, the exact ICE is
because of:
slen.1 = 20;
static character(kind=1) us[1][1:20] = {"foo "};
x.u = 0B;
x._u_length = 0;
{
struct t t.0;
struct t D.4308;
{
integer(kind=8) slen.1;
slen.1 = 20;
t.0.u = (character(kind=1)[1:0] *) __builtin_malloc (MAX_EXPR <(sizetype)
slen.1, 1>);
t.0._u_length = slen.1;
(void) __builtin_memcpy ((void *) t.0.u, (void *) &us[0], 20);
}
where the first slen.1 = 20; is emitted because it sees us has a VAR_DECL
ts.u.cl->backend_decl and so it wants to initialize it to the actual length.
This is invalid GENERIC, because the slen.1 variable is only declared inside
of a {} later on and so uses outside of it are wrong. Similarly wrong would
be if it is used later on. E.g. in the same testcase if it has
type(T) :: x, y
x = T(u = us(1))
y%u = us(1)
then there is
{
integer(kind=8) slen.1;
slen.1 = 20;
t.0.u = (character(kind=1)[1:0] *) __builtin_malloc (MAX_EXPR <(sizetype)
slen.1, 1>);
t.0._u_length = slen.1;
(void) __builtin_memcpy ((void *) t.0.u, (void *) &us[0], 20);
}
...
if (y.u != 0B) goto L.1;
y.u = (character(kind=1)[1:0] *) __builtin_malloc (MAX_EXPR <(sizetype)
slen.1, 1>
[PATCH wwwdocs] CSS: Color markup for /
In addition to underlines and strikethroughs. This makes it easier to
spot the differences in example code changes.
---
htdocs/gcc.css | 4
1 file changed, 4 insertions(+)
diff --git a/htdocs/gcc.css b/htdocs/gcc.css
index 77d01ee0..e32c4b93 100644
--- a/htdocs/gcc.css
+++ b/htdocs/gcc.css
@@ -98,6 +98,10 @@ div.copyright p:nth-child(3) { margin-bottom: 0; }
.blue{ color:blue; }
.blackbg { color:white; background-color: #00; }
+/* Inline markup for differences. */
+ins { background-color: lightgreen }
+del { background-color: pink }
+
/* Quote an e-mail. The first has the sender, the second the quote. */
blockquote.mail div:nth-child(2) { border-left: solid blue; padding-left: 4pt;
}
base-commit: 848ebfb46379178be3b0307c2ef99f4012e40edd
[PATCH wwwdocs] gcc-14: Add code examples for -Wreturn-mismatch
---
htdocs/gcc-14/porting_to.html | 46 ---
1 file changed, 43 insertions(+), 3 deletions(-)
diff --git a/htdocs/gcc-14/porting_to.html b/htdocs/gcc-14/porting_to.html
index bbbaa25a..123b5e9f 100644
--- a/htdocs/gcc-14/porting_to.html
+++ b/htdocs/gcc-14/porting_to.html
@@ -213,19 +213,59 @@ in functions which are declared to return
void, or
return statements without expressions for functions
returning a non-void type.
+
+Both function definitions below contain -Wreturn-mismatch
+errors:
+
+
+void
+do_something (int flag)
+{
+ if (!flag)
+return -1;
+ do_something_else ();
+}
+
+int
+unimplemented_function (void)
+{
+ puts ("unimplemented function foo called");
+}
+
+
+
To address this, remove the incorrect expression (or turn it into a
statement expression immediately prior to the return
statements if the expression has side effects), or add a dummy return
-value, as appropriate. If there is no suitable dummy return value,
-further changes may be needed to implement appropriate error handling.
+value, as appropriate.
+
+
+void
+do_something (int flag)
+{
+ if (!flag)
+return -1;
+ do_something_else ();
+}
+
+int
+unimplemented_function (void)
+{
+ puts ("unimplemented function foo called");
+ return 0;
+}
+
+
+If there is no suitable dummy return value, further changes may be
+needed to implement appropriate error handling.
Previously, these mismatches were diagnosed as
a -Wreturn-type warning. This warning still exists, and
is not treated as an error by default. It now covers remaining
potential correctness issues, such as reaching the closing
-brace } of function that does not
+brace } of a function that does not
return void.
base-commit: 5ef0adf3098478600f0c108e07e568d864b4c731
[PATCH wwwdocs] gcc-14: Some very common historic Autoconf probes that no longer work
---
htdocs/gcc-14/porting_to.html | 43 +++
1 file changed, 43 insertions(+)
diff --git a/htdocs/gcc-14/porting_to.html b/htdocs/gcc-14/porting_to.html
index 123b5e9f..ab65c5e7 100644
--- a/htdocs/gcc-14/porting_to.html
+++ b/htdocs/gcc-14/porting_to.html
@@ -437,6 +437,49 @@ issues addressed in more recent versions.) Versions
before 2.69 may
have generic probes (for example for standard C support) that rely on
C features that were removed in C99 and thus fail with GCC 14.
+
+Older Autoconf versions (for example, Autoconf 2.13) generate core
+probes that are incompatible with C99. These include the basic
+compiler functionality check:
+
+
+#include "confdefs.h"
+main(){return(0);}
+
+
+And a check for standard C compatibility:
+
+
+#define XOR(e, f) (((e) && !(f)) || (!(e) && (f)))
+int main () { int i; for (i = 0; i < 256; i++)
+if (XOR (islower (i), ISLOWER (i)) || toupper (i) != TOUPPER (i)) exit(2);
+exit (0); }
+
+
+(Several variants with different line breaks and whitespace were in
+use.) If it is not possible to port the configure script to current
+Autoconf, these issues can be patched directly:
+
+
+#include "confdefs.h"
+int main(){return(0);}
+
+
+And for the second probe:
+
+
+#define XOR(e, f) (((e) && !(f)) || (!(e) && (f)))
+int main () { int i; for (i = 0; i < 256; i++)
+if (XOR (islower (i), ISLOWER (i)) || toupper (i) != TOUPPER (i)) return
2;
+return 0; }
+
+
+There is a long tail of less frequent issues, involving keyword
+checking (for inline), and checks for dlopen
+and mmap. A setvbuf probe was previously
+expected to fail at run time because it triggered undefined behavior,
+now it fails because of a compilation error.
+
Turning errors back into warnings
base-commit: 1fcd61437d6a3d7bf24b993b09d525486dc9a2e5
Re: [PATCH v3] c++: wrong looser excep spec for dep noexcept [PR113158]
On 2/16/24 17:06, Marek Polacek wrote:
On Fri, Feb 16, 2024 at 04:39:47PM -0500, Patrick Palka wrote:
On Fri, 16 Feb 2024, Marek Polacek wrote:
+ /* We also have to defer checking when we're in a template and couldn't
+ instantiate & evaluate the noexcept to true/false. */
+ if (processing_template_decl)
+if ((base_throw
+&& (base_throw != noexcept_true_spec
+|| base_throw != noexcept_false_spec))
Shouldn't these innermost || be &&?
D'oh, yes, what a dumb mistake. But that shows that we could also just
always return true in a template ;).
Fixed. dg.exp passed so far.
OK.
-- >8 --
Here we find ourselves in maybe_check_overriding_exception_spec in
a template context where we can't instantiate a dependent noexcept.
That's OK, but we have to defer the checking otherwise we give wrong
errors.
PR c++/113158
gcc/cp/ChangeLog:
* search.cc (maybe_check_overriding_exception_spec): Defer checking
when a noexcept couldn't be instantiated & evaluated to false/true.
gcc/testsuite/ChangeLog:
* g++.dg/cpp0x/noexcept83.C: New test.
---
gcc/cp/search.cc| 11
gcc/testsuite/g++.dg/cpp0x/noexcept83.C | 37 +
2 files changed, 48 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/noexcept83.C
diff --git a/gcc/cp/search.cc b/gcc/cp/search.cc
index c948839dc53..827f48e8604 100644
--- a/gcc/cp/search.cc
+++ b/gcc/cp/search.cc
@@ -1975,6 +1975,17 @@ maybe_check_overriding_exception_spec (tree overrider,
tree basefn)
|| UNPARSED_NOEXCEPT_SPEC_P (over_throw))
return true;
+ /* We also have to defer checking when we're in a template and couldn't
+ instantiate & evaluate the noexcept to true/false. */
+ if (processing_template_decl)
+if ((base_throw
+&& base_throw != noexcept_true_spec
+&& base_throw != noexcept_false_spec)
+ || (over_throw
+ && over_throw != noexcept_true_spec
+ && over_throw != noexcept_false_spec))
+ return true;
+
if (!comp_except_specs (base_throw, over_throw, ce_derived))
{
auto_diagnostic_group d;
diff --git a/gcc/testsuite/g++.dg/cpp0x/noexcept83.C
b/gcc/testsuite/g++.dg/cpp0x/noexcept83.C
new file mode 100644
index 000..47832bbb44d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/noexcept83.C
@@ -0,0 +1,37 @@
+// PR c++/113158
+// { dg-do compile { target c++11 } }
+
+template
+struct V {
+ static constexpr bool t = false;
+};
+struct base {
+virtual int f() = 0;
+};
+
+template
+struct derived : base {
+int f() noexcept(V::t) override;
+};
+
+struct base2 {
+virtual int f() noexcept = 0;
+};
+
+template
+struct W {
+ static constexpr bool t = B;
+};
+
+template
+struct derived2 : base2 {
+int f() noexcept(W::t) override; // { dg-error "looser exception
specification" }
+};
+
+void
+g ()
+{
+ derived d1;
+ derived2 d2; // { dg-message "required from here" }
+ derived2 d3;
+}
base-commit: cd503b0616462445381a8232fb753239d319af76
Re: [patch, libgfortran] PR107068 Run-time error when reading logical arrays with a namelist
> OK for trunk? > I think simple enough to backport to 13 as well. OK, but probably best to wait a few weeks before backporting. FX
Re: [patch, libgfortran] Bug 105473 - semicolon allowed when list-directed read integer with decimal='point'
> OK for trunk and later backport to 13? OK. Thanks for the patch! FX
[PATCH v1] Internal-fn: Add new internal function SAT_ADDU
From: Pan Li
This patch would like to add the middle-end presentation for the
unsigned saturation add. Aka set the result of add to the max
when overflow. It will take the pattern similar as below.
SAT_ADDU (x, y) => (x + y) | (-(TYPE)((TYPE)(x + y) < x))
Take uint8_t as example, we will have:
* SAT_ADDU (1, 254) => 255.
* SAT_ADDU (1, 255) => 255.
* SAT_ADDU (2, 255) => 255.
* SAT_ADDU (255, 255) => 255.
The patch also implement the SAT_ADDU in the riscv backend as
the sample. Given below example:
uint64_t sat_add_u64 (uint64_t x, uint64_t y)
{
return (x + y) | (- (uint64_t)((uint64_t)(x + y) < x));
}
Before this patch:
uint64_t sat_add_uint64_t (uint64_t x, uint64_t y)
{
long unsigned int _1;
_Bool _2;
long unsigned int _3;
long unsigned int _4;
uint64_t _7;
long unsigned int _10;
__complex__ long unsigned int _11;
;; basic block 2, loop depth 0
;;pred: ENTRY
_11 = .ADD_OVERFLOW (x_5(D), y_6(D));
_1 = REALPART_EXPR <_11>;
_10 = IMAGPART_EXPR <_11>;
_2 = _10 != 0;
_3 = (long unsigned int) _2;
_4 = -_3;
_7 = _1 | _4;
return _7;
;;succ: EXIT
}
After this patch:
uint64_t sat_add_uint64_t (uint64_t x, uint64_t y)
{
uint64_t _7;
;; basic block 2, loop depth 0
;;pred: ENTRY
_7 = .SAT_ADDU (x_5(D), y_6(D)); [tail call]
return _7;
;;succ: EXIT
}
Then we will have the middle-end representation like .SAT_ADDU after
this patch.
PR target/51492
PR target/112600
gcc/ChangeLog:
* config/riscv/riscv-protos.h (riscv_expand_saturation_addu):
New func decl for the SAT_ADDU expand.
* config/riscv/riscv.cc (riscv_expand_saturation_addu): New func
impl for the SAT_ADDU expand.
* config/riscv/riscv.md (sat_addu_3): New pattern to impl
the standard name SAT_ADDU.
* doc/md.texi: Add doc for SAT_ADDU.
* internal-fn.cc (commutative_binary_fn_p): Add type IFN_SAT_ADDU.
* internal-fn.def (SAT_ADDU): Add SAT_ADDU.
* match.pd: Add simplify pattern patch for SAT_ADDU.
* optabs.def (OPTAB_D): Add sat_addu_optab.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/sat_addu-1.c: New test.
* gcc.target/riscv/sat_addu-2.c: New test.
* gcc.target/riscv/sat_addu-3.c: New test.
* gcc.target/riscv/sat_addu-4.c: New test.
* gcc.target/riscv/sat_addu-run-1.c: New test.
* gcc.target/riscv/sat_addu-run-2.c: New test.
* gcc.target/riscv/sat_addu-run-3.c: New test.
* gcc.target/riscv/sat_addu-run-4.c: New test.
* gcc.target/riscv/sat_arith.h: New test.
Signed-off-by: Pan Li
---
gcc/config/riscv/riscv-protos.h | 1 +
gcc/config/riscv/riscv.cc | 46 +
gcc/config/riscv/riscv.md | 11 +
gcc/doc/md.texi | 11 +
gcc/internal-fn.cc| 1 +
gcc/internal-fn.def | 1 +
gcc/match.pd | 22 +
gcc/optabs.def| 2 +
gcc/testsuite/gcc.target/riscv/sat_addu-1.c | 18 +++
gcc/testsuite/gcc.target/riscv/sat_addu-2.c | 20
gcc/testsuite/gcc.target/riscv/sat_addu-3.c | 17 +++
gcc/testsuite/gcc.target/riscv/sat_addu-4.c | 16 ++
.../gcc.target/riscv/sat_addu-run-1.c | 42
.../gcc.target/riscv/sat_addu-run-2.c | 42
.../gcc.target/riscv/sat_addu-run-3.c | 42
.../gcc.target/riscv/sat_addu-run-4.c | 49 +++
gcc/testsuite/gcc.target/riscv/sat_arith.h| 15 ++
17 files changed, 356 insertions(+)
create mode 100644 gcc/testsuite/gcc.target/riscv/sat_addu-1.c
create mode 100644 gcc/testsuite/gcc.target/riscv/sat_addu-2.c
create mode 100644 gcc/testsuite/gcc.target/riscv/sat_addu-3.c
create mode 100644 gcc/testsuite/gcc.target/riscv/sat_addu-4.c
create mode 100644 gcc/testsuite/gcc.target/riscv/sat_addu-run-1.c
create mode 100644 gcc/testsuite/gcc.target/riscv/sat_addu-run-2.c
create mode 100644 gcc/testsuite/gcc.target/riscv/sat_addu-run-3.c
create mode 100644 gcc/testsuite/gcc.target/riscv/sat_addu-run-4.c
create mode 100644 gcc/testsuite/gcc.target/riscv/sat_arith.h
diff --git a/gcc/config/riscv/riscv-protos.h b/gcc/config/riscv/riscv-protos.h
index ae1685850ac..f201b2384f9 100644
--- a/gcc/config/riscv/riscv-protos.h
+++ b/gcc/config/riscv/riscv-protos.h
@@ -132,6 +132,7 @@ extern void riscv_asm_output_external (FILE *, const tree,
const char *);
extern bool
riscv_zcmp_valid_stack_adj_bytes_p (HOST_WIDE_INT, int);
extern void riscv_legitimize_poly_move (machine_mode, rtx, rtx, rtx);
+extern void riscv_expand_saturation_addu (rtx, rtx, rtx);
#ifdef RTX_CODE
extern void riscv_expand_int_scc (rtx, enum rtx_code, rtx, rtx, bool
*invert_ptr = 0);
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index
Re: [PATCH][_GLIBCXX_DEBUG] Fix std::__niter_base behavior
Thanks for the link, tested and committed.
On 15/02/2024 19:40, Jonathan Wakely wrote:
On Thu, 15 Feb 2024 at 18:38, François Dumont
wrote:
On 15/02/2024 14:17, Jonathan Wakely wrote:
On Wed, 14 Feb 2024 at 21:48, François Dumont
wrote:
On 14/02/2024 20:44, Jonathan Wakely wrote:
On Wed, 14 Feb 2024 at 18:39, François Dumont
wrote:
libstdc++: [_GLIBCXX_DEBUG] Fix std::__niter_base behavior
std::__niter_base is used in _GLIBCXX_DEBUG mode to
remove _Safe_iterator<>
wrapper on random access iterators. But doing so it
should also preserve
original
behavior to remove __normal_iterator wrapper.
libstdc++-v3/ChangeLog:
* include/bits/stl_algobase.h (std::__niter_base):
Redefine the
overload
definitions for __gnu_debug::_Safe_iterator.
* include/debug/safe_iterator.tcc
(std::__niter_base): Adapt
declarations.
Ok to commit once all tests completed (still need to
check pre-c++11) ?
The declaration in include/bits/stl_algobase.h has a
noexcept-specifier but the definition in
include/debug/safe_iterator.tcc does not have one - that
seems wrong (I'm surprised it even compiles).
It does !
The diagnostic is suppressed without -Wsystem-headers:
/home/jwakely/gcc/14/include/c++/14.0.1/debug/safe_iterator.tcc:255:5:warning:
declaration of 'template constexpr
decltype (std::__
niter_base(declval<_Ite>())) std::__niter_base(const
__gnu_debug::_Safe_iterator<_Iterator, _Sequence,
random_access_iterator_tag>&)' has a different except
ion specifier [-Wsystem-headers]
255 | __niter_base(const ::__gnu_debug::_Safe_iterator<_Ite, _Seq,
| ^~~~
/home/jwakely/gcc/14/include/c++/14.0.1/bits/stl_algobase.h:335:5:note:
from previous declaration 'template
constexpr decltype (std
::__niter_base(declval<_Ite>())) std::__niter_base(const
__gnu_debug::_Safe_iterator<_Iterator, _Sequence,
random_access_iterator_tag>&) noexcept (noexcept
(is_nothrow_copy_constructible()))>::value))'
335 | __niter_base(const ::__gnu_debug::_Safe_iterator<_Ite, _Seq,
| ^~~~
It's a hard error with Clang though:
deb.cc:7:10: error: call to '__niter_base' is ambiguous
Yes, I eventually got the error too, I hadn't run enough tests yet.
I thought it was only necessary at declaration, and I also
had troubles doing it right at definition because of the
interaction with the auto and ->.
The trailing-return-type has to come after the noexcept-specifier.
Now simplified and consistent in this new proposal.
Just using std::is_nothrow_copy_constructible<_Ite> seems
simpler, that will be true for __normal_iterator if
is_nothrow_copy_constructible is true.
Ok
The definition in include/debug/safe_iterator.tcc should use
std::declval<_Ite>() not declval<_Ite>(). Is there any
reason why the definition uses a late-specified-return-type
(i.e. auto and ->) when the declaration doesn't?
I initially plan to use '->
std::decltype(std::__niter_base(__it.base()))' but this did
not compile, ambiguity issue. So I resort to using
std::declval and I could have then done it the same way as
declaration, done now.
Attached is what I'm testing, ok to commit once fully tested ?
OK, thanks.
Thanks for validation but I have a problem to test for c++98.
When I do:
make CXXFLAGS=-std=c++98 check-debug
That doesn't work any more, see
https://gcc.gnu.org/onlinedocs/libstdc++/manual/test.html#test.run.permutations
I see in debug/libstdc++.log for example:
Executing on host: /home/fdumont/dev/gcc/build/./gcc/xg++
-shared-libgcc ... -mshstk -std=c++98 -g -O2 -DLOCALEDIR="."
-nostdinc++ -I/home/fdumont/dev/gcc/...
/home/fdumont/dev/gcc/git/libstdc++-v3/testsuite/25_algorithms/copy/3.cc
-D_GLIBCXX_DEBUG -std=gnu++17 -include bits/stdc++.h ... -lm
-o ./3.exe (timeout = 360)
The -std=c++98 is there but later comes the -std=gnu++17 so I
think it runs in C++17, no ?
I also tried the documented alternative:
make check
'RUNTESTFLAGS=--target_board=unix/-O3\"{-std=gnu++98,-std=gnu++11,-std=gnu++14}\"'
but same problem, -std=gnu++17 comes last.
I'll try to rebuild all from scratch but I won't commit soon then.
Re: [PATCH] fortran: gfc_trans_subcomponent_assign fixes [PR113503]
Hi Jakub,
On 2/17/24 10:02, Jakub Jelinek wrote:
Hi!
The r14-870 changes broke xtb package tests (reduced testcase is the first
one below) and caused ICEs on a test derived from that (the second one).
[...]
thanks for your detailed analysis and for the patch, which puts
things in straight order to actually fix two issues here!
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK for trunk, except for the minor nit below.
--- gcc/testsuite/gfortran.dg/pr113503_1.f90.jj 2024-02-16 14:16:17.937153094
+0100
+++ gcc/testsuite/gfortran.dg/pr113503_1.f902024-02-16 14:16:10.124258815
+0100
@@ -0,0 +1,18 @@
+! PR fortran/113503
+! { dg-do compile }
+! { dg-options "-O2 -fno-inline -Wuninitialized" }
+
+program pr113503
+ implicit none
+ type :: T
+character(len=:), allocatable :: u
+ end type
+ character(len=20) :: us(1) = 'foobar'
+ type(T) :: x
+ x = T(u = trim (us(1))) ! { dg-bogus "is used uninitialized" }
tab here not allowed in Fortran
My newsreader shows a tab here, giving a warning when running the test.
Also, applying your patch on top of r14-9045 I do not see the
uninitialized warning, which could have been fixed by r14-8947.
Please recheck and adjust accordingly.
+ call foo
+contains
+ subroutine foo
+if (x%u /= 'foobar') stop 1
+ end subroutine
+end
Thanks,
Harald
Re: [PATCH] fortran: gfc_trans_subcomponent_assign fixes [PR113503]
On Sat, Feb 17, 2024 at 04:05:16PM +0100, Harald Anlauf wrote:
> > +program pr113503
> > + implicit none
> > + type :: T
> > +character(len=:), allocatable :: u
> > + end type
> > + character(len=20) :: us(1) = 'foobar'
> > + type(T) :: x
> > + x = T(u = trim (us(1))) ! { dg-bogus "is used uninitialized" }
> tab here not allowed in Fortran
>
> My newsreader shows a tab here, giving a warning when running the test.
> Also, applying your patch on top of r14-9045 I do not see the
> uninitialized warning, which could have been fixed by r14-8947.
> Please recheck and adjust accordingly.
I certainly see if I apply just the testsuite part of the patch to current
trunk
make check-gfortran RUNTESTFLAGS='--target_board=unix\{-m32,-m64\}
dg.exp=pr113503*.f90'
...
FAIL: gfortran.dg/pr113503_1.f90 -O (test for bogus messages, line 12)
FAIL: gfortran.dg/pr113503_2.f90 -O (internal compiler error: in
gimplify_var_or_parm_decl, at gimplify.cc:3280)
FAIL: gfortran.dg/pr113503_2.f90 -O (test for excess errors)
=== gfortran Summary for unix/-m32 ===
# of expected passes1
# of unexpected failures3
...
FAIL: gfortran.dg/pr113503_1.f90 -O (test for bogus messages, line 12)
FAIL: gfortran.dg/pr113503_2.f90 -O (internal compiler error: in
gimplify_var_or_parm_decl, at gimplify.cc:3280)
FAIL: gfortran.dg/pr113503_2.f90 -O (test for excess errors)
=== gfortran Summary for unix/-m64 ===
# of expected passes1
# of unexpected failures3
and that is all gone after I apply the trans-expr.cc patch as well and
make before the above command.
I have replaced the tab with a space, but from what I can see, there is no
warning/error with the options it is compiled, warning with -Wtabs or -Wall
and error with -pedantic-errors, but those options aren't used.
Thanks.
Jakub
Re: [PATCH] libgccjit: Add type checks in gcc_jit_block_add_assignment_op
David: Ping. On Thu, 2023-12-21 at 08:36 -0500, Antoni Boucher wrote: > Hi. > Here's the updated patch. > Thanks. > > On Thu, 2023-12-07 at 20:15 -0500, David Malcolm wrote: > > On Thu, 2023-12-07 at 17:34 -0500, Antoni Boucher wrote: > > > Hi. > > > This patch adds checks gcc_jit_block_add_assignment_op to make > > > sure > > > it > > > is only ever called on numeric types. > > > > > > With the previous patch, this might require a change to also > > > allow > > > vector types here. > > > > > > Thanks for the review. > > > > Thanks for the patch. > > > > [...snip...] > > > > > @@ -2890,6 +2900,17 @@ gcc_jit_block_add_assignment_op > > > (gcc_jit_block *block, > > > lvalue->get_type ()->get_debug_string (), > > > rvalue->get_debug_string (), > > > rvalue->get_type ()->get_debug_string ()); > > > + // TODO: check if it is a numeric vector? > > > + RETURN_IF_FAIL_PRINTF3 ( > > > + lvalue->get_type ()->is_numeric () && rvalue->get_type ()- > > > > is_numeric (), ctxt, loc, > > > + "gcc_jit_block_add_assignment_op %s has non-numeric lvalue > > > %s > > > (type: %s)", > > > + gcc::jit::binary_op_reproducer_strings[op], > > > + lvalue->get_debug_string (), lvalue->get_type ()- > > > > get_debug_string ()); > > > > The condition being tested here should probably just be: > > > > lvalue->get_type ()->is_numeric () > > > > since otherwise if the lvalue's type is numeric and the rvalue's > > type > > fails to be, then the user would incorrectly get a message about > > the > > lvalue. > > > > > + RETURN_IF_FAIL_PRINTF3 ( > > > + rvalue->get_type ()->is_numeric () && rvalue->get_type ()- > > > > is_numeric (), ctxt, loc, > > > + "gcc_jit_block_add_assignment_op %s has non-numeric rvalue > > > %s > > > (type: %s)", > > > + gcc::jit::binary_op_reproducer_strings[op], > > > + rvalue->get_debug_string (), rvalue->get_type ()- > > > > get_debug_string ()); > > > > The condition being tested here seems to have a redundant repeated: > > && rvalue->get_type ()->is_numeric () > > > > Am I missing something, or is that a typo? > > > > [...snip...] > > > > The patch is OK otherwise. > > > > Thanks > > Dave > > > > > > >
Re: [PATCH] libgccjit: Add vector permutation and vector access operations
David: Ping.
On Fri, 2024-01-19 at 15:21 -0500, Antoni Boucher wrote:
> David: Ping.
>
> On Thu, 2023-11-30 at 17:16 -0500, Antoni Boucher wrote:
> > All of these are fixed in this new patch.
> > Thanks for the review.
> >
> > On Mon, 2023-11-20 at 18:05 -0500, David Malcolm wrote:
> > > On Fri, 2023-11-17 at 17:36 -0500, Antoni Boucher wrote:
> > > > Hi.
> > > > This patch adds a vector permutation and vector access
> > > > operations
> > > > (bug
> > > > 112602).
> > > >
> > > > This was split from this patch:
> > > > https://gcc.gnu.org/pipermail/jit/2023q1/001606.html
> > > >
> > >
> > > Thanks for the patch.
> > >
> > > Overall, looks good, but 3 minor nitpicks:
> > >
> > > [...snip...]
> > >
> > > > diff --git a/gcc/jit/docs/topics/compatibility.rst
> > > > b/gcc/jit/docs/topics/compatibility.rst
> > > > index ebede440ee4..a764e3968d1 100644
> > > > --- a/gcc/jit/docs/topics/compatibility.rst
> > > > +++ b/gcc/jit/docs/topics/compatibility.rst
> > > > @@ -378,3 +378,13 @@ alignment of a variable:
> > > >
> > > > ``LIBGCCJIT_ABI_25`` covers the addition of
> > > > :func:`gcc_jit_type_get_restrict`
> > > > +
> > > > +
> > > > +.. _LIBGCCJIT_ABI_26:
> > > > +
> > > > +``LIBGCCJIT_ABI_26``
> > > > +
> > > > +``LIBGCCJIT_ABI_26`` covers the addition of functions to
> > > > manipulate vectors:
> > > > +
> > > > + * :func:`gcc_jit_context_new_rvalue_vector_perm`
> > > > + * :func:`gcc_jit_context_new_vector_access`
> > > > diff --git a/gcc/jit/docs/topics/expressions.rst
> > > > b/gcc/jit/docs/topics/expressions.rst
> > > > index 42cfee36302..4a45aa13f5c 100644
> > > > --- a/gcc/jit/docs/topics/expressions.rst
> > > > +++ b/gcc/jit/docs/topics/expressions.rst
> > > > @@ -295,6 +295,35 @@ Vector expressions
> > > >
> > > > #ifdef
> > > > LIBGCCJIT_HAVE_gcc_jit_context_new_rvalue_from_vector
> > > >
> > > > +.. function:: gcc_jit_rvalue * \
> > > > + gcc_jit_context_new_rvalue_vector_perm
> > > > (gcc_jit_context *ctxt, \
> > > > +
> > > > gcc_jit_location *loc, \
> > > > +
> > > > gcc_jit_rvalue *elements1, \
> > > > +
> > > > gcc_jit_rvalue *elements2, \
> > > > +
> > > > gcc_jit_rvalue *mask);
> > > > +
> > > > + Build a permutation of two vectors.
> > > > +
> > > > + "elements1" and "elements2" should have the same type.
> > > > + The length of "mask" and "elements1" should be the same.
> > > > + The element type of "mask" should be integral.
> > > > + The size of the element type of "mask" and "elements1"
> > > > should
> > > > be the same.
> > > > +
> > > > + This entrypoint was added in :ref:`LIBGCCJIT_ABI_25`; you
> > > > can
> > > > test for
> > > ^^
> > > Should be 26
> > >
> > > [...snip...]
> > >
> > > > Unary Operations
> > > >
> > > >
> > > > @@ -1020,3 +1049,27 @@ Field access is provided separately for
> > > > both
> > > > lvalues and rvalues.
> > > > PTR[INDEX]
> > > >
> > > > in C (or, indeed, to ``PTR + INDEX``).
> > > > +
> > > > +.. function:: gcc_jit_lvalue *\
> > > > + gcc_jit_context_new_vector_access
> > > > (gcc_jit_context
> > > > *ctxt,\
> > > > +
> > > > gcc_jit_location
> > > > *loc,\
> > > > +
> > > > gcc_jit_rvalue
> > > > *vector,\
> > > > +
> > > > gcc_jit_rvalue
> > > > *index)
> > > > +
> > > > + Given an rvalue of vector type ``T __attribute__
> > > > ((__vector_size__ (SIZE)))``, get the element `T` at
> > > > + the given index.
> > > > +
> > > > + This entrypoint was added in :ref:`LIBGCCJIT_ABI_25`; you
> > > > can
> > > > test for
> > > ^^
> > >
> > > Likewise here.
> > >
> > > [...snip...]
> > >
> > > > @@ -4071,6 +4107,79 @@ gcc_jit_context_new_rvalue_from_vector
> > > > (gcc_jit_context *ctxt,
> > > > (gcc::jit::recording::rvalue **)elements);
> > > > }
> > > >
> > > > +/* Public entrypoint. See description in libgccjit.h.
> > > > +
> > > > + After error-checking, the real work is done by the
> > > > + gcc::jit::recording::context::new_rvalue_vector_perm
> > > > method,
> > > > in
> > > > + jit-recording.cc. */
> > > > +
> > > > +gcc_jit_rvalue *
> > > > +gcc_jit_context_new_rvalue_vector_perm (gcc_jit_context *ctxt,
> > > > + gcc_jit_location *loc,
> > > > + gcc_jit_rvalue
> > > > *elements1,
> > > > + gcc_jit_rvalue
> > > > *elements2,
> > > > + gcc_jit_rvalue *mask)
> > > > +{
> > > > + RETURN
Re: [PATCH] libgccjit: Allow sending a const pointer as argument
David: Ping. On Fri, 2024-01-19 at 15:59 -0500, Antoni Boucher wrote: > David: Ping. > > On Thu, 2023-12-21 at 11:59 -0500, Antoni Boucher wrote: > > Hi. > > This patch adds the ability to send const pointer as argument to a > > function. > > Thanks for the review. >
Re: [PATCH] libgccjit: Fix float playback for cross-compilation
David: Ping. On Thu, 2024-01-25 at 16:04 -0500, Antoni Boucher wrote: > Thanks for the review! > > On Wed, 2024-01-24 at 13:10 -0500, David Malcolm wrote: > > On Thu, 2024-01-11 at 18:42 -0500, Antoni Boucher wrote: > > > Hi. > > > This patch fixes the bug 113343. > > > I'm wondering if there's a better solution than using mpfr. > > > The only other solution I found is real_from_string, but that > > > seems > > > overkill to convert the number to a string. > > > I could not find a better way to create a real value from a host > > > double. > > > > I took a look, and I don't see a better way; it seems weird to go > > through a string stage. Ideally there would be a > > real_from_host_double, but I don't see one. > > > > Is there a cross-platform way to directly access the representation > > of > > a host double? > > I have no idea. > > > > > > If there's no solution, do we lose some precision by using mpfr? > > > Running Rust's core library tests, there was a difference of one > > > decimal, so I'm wondering if there's some lost precision, or if > > > it's > > > just because those tests don't work on m68k which was my test > > > target. > > > > Sorry, can you clarify what you mean by "a difference of one > > decimal" > > above? > > Let's say the Rust core tests expected the value "1.23456789", it > instead got the value "1.2345678" (e.g. without the last decimal). > Not sure if this is expected. > Everything works fine for x86-64; this just happened for m68k which > is > not well supported for now in Rust, so that might just be that the > test > doesn't work on this platform. > > > > > > Also, I'm not sure how to write a test this fix. Any ideas? > > > > I think we don't need cross-compilation-specific tests, we should > > just > > use and/or extend the existing coverage for > > gcc_jit_context_new_rvalue_from_double e.g. in test-constants.c and > > test-types.c > > > > We probably should have test coverage for "awkward" values; we > > already > > have coverage for DBL_MIN and DBL_MAX, but we don't yet have test > > coverage for: > > * quiet/signaling NaN > > * +ve/-ve inf > > * -ve zero > > Is this something you would want for this patch? > > > > > Thanks > > Dave > > >
Re: [PATCH] libgccjit: Add gcc_jit_global_set_readonly
David: Ping. On Wed, 2024-01-24 at 10:36 -0500, Antoni Boucher wrote: > Yes, it is for a use case inside of rustc_codegen_gcc. > The compiler is structured in a way where we don't know if a global > variable might be constant when it is created. > > On Wed, 2024-01-24 at 10:09 -0500, David Malcolm wrote: > > On Fri, 2024-01-19 at 16:57 -0500, Antoni Boucher wrote: > > > Hi. > > > This patch adds a new API gcc_jit_global_set_readonly: it's > > > equivalent > > > to having a const global variable, but it is useful in the case > > > of > > > complex compilers where it is not convenient to use const. > > > Thanks for the review. > > > > Hi Antoni; thanks for the patch. > > > > Can you give an example of where/why this might be used? > > Presumably this is motivated by a use case you had inside the rustc > > backend? > > > > Thanks > > Dave > > >
Re: [PATCH wwwdocs] CSS: Color markup for /
On Sat, 17 Feb 2024, Florian Weimer wrote: > In addition to underlines and strikethroughs. This makes it easier to > spot the differences in example code changes. Looks like a good idea! Thanks, Gerald
Re: [PATCH] testsuite: Disable slow and unneeded test variants
On Fri, Feb 16, 2024 at 07:06:57PM +0100, Jakub Jelinek wrote:
> On Fri, Feb 16, 2024 at 07:52:17PM +0200, Dimitar Dimitrov wrote:
> > The issue in PR112344 is triggered only at -O2, so there is little value
> > in running the test at lower optimization levels. At the same time the
>
> That is generally not true.
> We had hundreds of cases in the history where a test written for one bug
> let us discover a different bug later on, often at different optimization
> level etc.
>
> If the test is too slow, perhaps the dg-skip-if should take the
> run_expensive_tests effective target into account, like:
> /* { dg-skip-if "" { ! run_expensive_tests } { "*" } { "-O0" "-O1" } } */
Thank you, this is reasonable. I'll send V2 with that change.
Curiously, -Os also needs to be added to the slow variants, along with
-O0 and -O1.
>
> But guess another question is if the bug can be reproduced with fewer
> iterations...
I could not think of a way to do it.
Regards,
Dimitar
>
> Jakub
>
Re: [PATCH] testsuite: Disable slow and unneeded test variants
On Sat, Feb 17, 2024 at 09:59:12PM +0200, Dimitar Dimitrov wrote:
> On Fri, Feb 16, 2024 at 07:06:57PM +0100, Jakub Jelinek wrote:
> > On Fri, Feb 16, 2024 at 07:52:17PM +0200, Dimitar Dimitrov wrote:
> > > The issue in PR112344 is triggered only at -O2, so there is little value
> > > in running the test at lower optimization levels. At the same time the
> >
> > That is generally not true.
> > We had hundreds of cases in the history where a test written for one bug
> > let us discover a different bug later on, often at different optimization
> > level etc.
> >
> > If the test is too slow, perhaps the dg-skip-if should take the
> > run_expensive_tests effective target into account, like:
> > /* { dg-skip-if "" { ! run_expensive_tests } { "*" } { "-O0" "-O1" } } */
>
> Thank you, this is reasonable. I'll send V2 with that change.
>
> Curiously, -Os also needs to be added to the slow variants, along with
> -O0 and -O1.
Ok then.
> > But guess another question is if the bug can be reproduced with fewer
> > iterations...
>
> I could not think of a way to do it.
Yeah, I haven't managed to reduce the number of iterations and still trigger
the bug.
Jakub
[PATCH v2] testsuite: Mark non-optimized variants as expensive
When not optimized for speed, the test for PR112344 takes several
seconds to execute on native x86_64, and 15 minutes on PRU target
simulator. Thus mark those variants as expensive. The -O2 variant
which originally triggered the PR is not expensive, hence it is
still run by default.
Ok for trunk?
PR middle-end/112344
gcc/testsuite/ChangeLog:
* gcc.dg/torture/pr112344.c: Run non-optimized variants only
if expensive tests are allowed.
Signed-off-by: Dimitar Dimitrov
---
Changes since V1:
- Mark as expensive instead of outright disabling variants
which are not optimized for speed.
gcc/testsuite/gcc.dg/torture/pr112344.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/gcc/testsuite/gcc.dg/torture/pr112344.c
b/gcc/testsuite/gcc.dg/torture/pr112344.c
index c52d2c8304b..657322caed0 100644
--- a/gcc/testsuite/gcc.dg/torture/pr112344.c
+++ b/gcc/testsuite/gcc.dg/torture/pr112344.c
@@ -1,5 +1,6 @@
/* { dg-do run } */
/* { dg-require-effective-target int32plus } */
+/* { dg-skip-if "non-optimized code is too slow" { ! run_expensive_tests } {
"*" } { "-O2" "-O3" } } */
int
main ()
--
2.43.0
Re: [PATCH v2] testsuite: Mark non-optimized variants as expensive
On Sat, Feb 17, 2024 at 10:15:55PM +0200, Dimitar Dimitrov wrote: > When not optimized for speed, the test for PR112344 takes several > seconds to execute on native x86_64, and 15 minutes on PRU target > simulator. Thus mark those variants as expensive. The -O2 variant > which originally triggered the PR is not expensive, hence it is > still run by default. > > Ok for trunk? > > PR middle-end/112344 > > gcc/testsuite/ChangeLog: > > * gcc.dg/torture/pr112344.c: Run non-optimized variants only > if expensive tests are allowed. Ok, thanks. Jakub
Re: [PATCH wwwdocs] gcc-14: Some very common historic Autoconf probes that no longer work
Florian Weimer writes:
> ---
> htdocs/gcc-14/porting_to.html | 43
> +++
> 1 file changed, 43 insertions(+)
>
> diff --git a/htdocs/gcc-14/porting_to.html b/htdocs/gcc-14/porting_to.html
> index 123b5e9f..ab65c5e7 100644
> --- a/htdocs/gcc-14/porting_to.html
> +++ b/htdocs/gcc-14/porting_to.html
> @@ -437,6 +437,49 @@ issues addressed in more recent versions.) Versions
> before 2.69 may
> have generic probes (for example for standard C support) that rely on
> C features that were removed in C99 and thus fail with GCC 14.
>
> +
> +Older Autoconf versions (for example, Autoconf 2.13) generate core
> +probes that are incompatible with C99. These include the basic
> +compiler functionality check:
> +
> +
> +#include "confdefs.h"
> +main(){return(0);}
> +
> +
> +And a check for standard C compatibility:
> +
> +
> +#define XOR(e, f) (((e) && !(f)) || (!(e) && (f)))
> +int main () { int i; for (i = 0; i < 256; i++)
> +if (XOR (islower (i), ISLOWER (i)) || toupper (i) != TOUPPER (i)) exit(2);
> +exit (0); }
> +
> +
Consider adding the check name so it shows up on search engine results?
("Checking for $X ... no")
> +(Several variants with different line breaks and whitespace were in
> +use.) If it is not possible to port the configure script to current
> +Autoconf, these issues can be patched directly:
> +
> +
> +#include "confdefs.h"
> +int main(){return(0);}
> +
> +
> +And for the second probe:
> +
> +
> +#define XOR(e, f) (((e) && !(f)) || (!(e) && (f)))
> +int main () { int i; for (i = 0; i < 256; i++)
> +if (XOR (islower (i), ISLOWER (i)) || toupper (i) != TOUPPER (i))
> return 2;
> +return 0; }
> +
> +
> +There is a long tail of less frequent issues, involving keyword
> +checking (for inline), and checks for dlopen
> +and mmap. A setvbuf probe was previously
> +expected to fail at run time because it triggered undefined behavior,
> +now it fails because of a compilation error.
> +
> Turning errors back into warnings
>
>
LGTM otherwise.
>
> base-commit: 1fcd61437d6a3d7bf24b993b09d525486dc9a2e5
signature.asc
Description: PGP signature
Re: [PATCH wwwdocs] gcc-14: Add code examples for -Wreturn-mismatch
Florian Weimer writes:
> ---
> htdocs/gcc-14/porting_to.html | 46
> ---
> 1 file changed, 43 insertions(+), 3 deletions(-)
>
> diff --git a/htdocs/gcc-14/porting_to.html b/htdocs/gcc-14/porting_to.html
> index bbbaa25a..123b5e9f 100644
> --- a/htdocs/gcc-14/porting_to.html
> +++ b/htdocs/gcc-14/porting_to.html
> @@ -213,19 +213,59 @@ in functions which are declared to return
> void, or
> return statements without expressions for functions
> returning a non-void type.
>
> +
> +Both function definitions below contain -Wreturn-mismatch
> +errors:
> +
> +
> +void
> +do_something (int flag)
> +{
> + if (!flag)
> +return -1;
> + do_something_else ();
> +}
> +
> +int
> +unimplemented_function (void)
> +{
> + puts ("unimplemented function foo called");
> +}
> +
> +
> +
>
> To address this, remove the incorrect expression (or turn it into a
> statement expression immediately prior to the return
> statements if the expression has side effects), or add a dummy return
> -value, as appropriate. If there is no suitable dummy return value,
> -further changes may be needed to implement appropriate error handling.
> +value, as appropriate.
> +
> +
> +void
> +do_something (int flag)
> +{
> + if (!flag)
> +return -1;
> + do_something_else ();
> +}
> +
> +int
> +unimplemented_function (void)
> +{
> + puts ("unimplemented function foo called");
> + return 0;
> +}
> +
> +
> +If there is no suitable dummy return value, further changes may be
> +needed to implement appropriate error handling.
LGTM.
>
>
> Previously, these mismatches were diagnosed as
> a -Wreturn-type warning. This warning still exists, and
> is not treated as an error by default. It now covers remaining
> potential correctness issues, such as reaching the closing
> -brace } of function that does not
> +brace } of a function that does not
> return void.
>
>
>
> base-commit: 5ef0adf3098478600f0c108e07e568d864b4c731
signature.asc
Description: PGP signature
New Chinese (simplified) PO file for 'gcc' (version 13.2.0)
Hello, gentle maintainer. This is a message from the Translation Project robot. A revised PO file for textual domain 'gcc' has been submitted by the Chinese (simplified) team of translators. The file is available at: https://translationproject.org/latest/gcc/zh_CN.po (This file, 'gcc-13.2.0.zh_CN.po', has just now been sent to you in a separate email.) All other PO files for your package are available in: https://translationproject.org/latest/gcc/ Please consider including all of these in your next release, whether official or a pretest. Whenever you have a new distribution with a new version number ready, containing a newer POT file, please send the URL of that distribution tarball to the address below. The tarball may be just a pretest or a snapshot, it does not even have to compile. It is just used by the translators when they need some extra translation context. The following HTML page has been updated: https://translationproject.org/domain/gcc.html If any question arises, please contact the translation coordinator. Thank you for all your work, The Translation Project robot, in the name of your translation coordinator.
Re:[pushed] [PATCH 1/2] LoongArch: Fix wrong return value type of __iocsrrd_h.
Pushed to r14-9053.
在 2024/2/6 上午10:10, Lulu Cheng 写道:
gcc/ChangeLog:
* config/loongarch/larchintrin.h (__iocsrrd_h): Modify the
function return value type to unsigned short.
---
gcc/config/loongarch/larchintrin.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gcc/config/loongarch/larchintrin.h
b/gcc/config/loongarch/larchintrin.h
index ff2c9f460ac..04672e71728 100644
--- a/gcc/config/loongarch/larchintrin.h
+++ b/gcc/config/loongarch/larchintrin.h
@@ -268,7 +268,7 @@ __iocsrrd_b (unsigned int _1)
/* Assembly instruction format: rd, rj. */
/* Data types in instruction templates: UHI, USI. */
-extern __inline unsigned char
+extern __inline unsigned short
__attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
__iocsrrd_h (unsigned int _1)
{
Re:[pushed] [PATCH 2/2] LoongArch: Remove redundant symbol type conversions in larchintrin.h.
Pushed to r14-9054.
在 2024/2/6 上午10:10, Lulu Cheng 写道:
gcc/ChangeLog:
* config/loongarch/larchintrin.h (__movgr2fcsr): Remove redundant
symbol type conversions.
(__cacop_d): Likewise.
(__cpucfg): Likewise.
(__asrtle_d): Likewise.
(__asrtgt_d): Likewise.
(__lddir_d): Likewise.
(__ldpte_d): Likewise.
(__crc_w_b_w): Likewise.
(__crc_w_h_w): Likewise.
(__crc_w_w_w): Likewise.
(__crc_w_d_w): Likewise.
(__crcc_w_b_w): Likewise.
(__crcc_w_h_w): Likewise.
(__crcc_w_w_w): Likewise.
(__crcc_w_d_w): Likewise.
(__csrrd_w): Likewise.
(__csrwr_w): Likewise.
(__csrxchg_w): Likewise.
(__csrrd_d): Likewise.
(__csrwr_d): Likewise.
(__csrxchg_d): Likewise.
(__iocsrrd_b): Likewise.
(__iocsrrd_h): Likewise.
(__iocsrrd_w): Likewise.
(__iocsrrd_d): Likewise.
(__iocsrwr_b): Likewise.
(__iocsrwr_h): Likewise.
(__iocsrwr_w): Likewise.
(__iocsrwr_d): Likewise.
(__frecipe_s): Likewise.
(__frecipe_d): Likewise.
(__frsqrte_s): Likewise.
(__frsqrte_d): Likewise.
---
gcc/config/loongarch/larchintrin.h | 69 ++
1 file changed, 33 insertions(+), 36 deletions(-)
diff --git a/gcc/config/loongarch/larchintrin.h
b/gcc/config/loongarch/larchintrin.h
index 04672e71728..0f55bdae838 100644
--- a/gcc/config/loongarch/larchintrin.h
+++ b/gcc/config/loongarch/larchintrin.h
@@ -87,13 +87,13 @@ __rdtimel_w (void)
/* Assembly instruction format: fcsr, rj. */
/* Data types in instruction templates: VOID, UQI, USI. */
#define __movgr2fcsr(/*ui5*/ _1, _2) \
- __builtin_loongarch_movgr2fcsr ((_1), (unsigned int) _2);
+ __builtin_loongarch_movgr2fcsr ((_1), _2);
#if defined __loongarch64
/* Assembly instruction format: ui5, rj, si12. */
/* Data types in instruction templates: VOID, USI, UDI, SI. */
#define __cacop_d(/*ui5*/ _1, /*unsigned long int*/ _2, /*si12*/ _3) \
- ((void) __builtin_loongarch_cacop_d ((_1), (unsigned long int) (_2), (_3)))
+ __builtin_loongarch_cacop_d ((_1), (_2), (_3))
#else
#error "Unsupported ABI."
#endif
@@ -104,7 +104,7 @@ extern __inline unsigned int
__attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
__cpucfg (unsigned int _1)
{
- return (unsigned int) __builtin_loongarch_cpucfg ((unsigned int) _1);
+ return __builtin_loongarch_cpucfg (_1);
}
#ifdef __loongarch64
@@ -114,7 +114,7 @@ extern __inline void
__attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
__asrtle_d (long int _1, long int _2)
{
- __builtin_loongarch_asrtle_d ((long int) _1, (long int) _2);
+ __builtin_loongarch_asrtle_d (_1, _2);
}
/* Assembly instruction format: rj, rk. */
@@ -123,7 +123,7 @@ extern __inline void
__attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
__asrtgt_d (long int _1, long int _2)
{
- __builtin_loongarch_asrtgt_d ((long int) _1, (long int) _2);
+ __builtin_loongarch_asrtgt_d (_1, _2);
}
#endif
@@ -131,7 +131,7 @@ __asrtgt_d (long int _1, long int _2)
/* Assembly instruction format: rd, rj, ui5. */
/* Data types in instruction templates: DI, DI, UQI. */
#define __lddir_d(/*long int*/ _1, /*ui5*/ _2) \
- ((long int) __builtin_loongarch_lddir_d ((long int) (_1), (_2)))
+ __builtin_loongarch_lddir_d ((_1), (_2))
#else
#error "Unsupported ABI."
#endif
@@ -140,7 +140,7 @@ __asrtgt_d (long int _1, long int _2)
/* Assembly instruction format: rj, ui5. */
/* Data types in instruction templates: VOID, DI, UQI. */
#define __ldpte_d(/*long int*/ _1, /*ui5*/ _2) \
- ((void) __builtin_loongarch_ldpte_d ((long int) (_1), (_2)))
+ __builtin_loongarch_ldpte_d ((_1), (_2))
#else
#error "Unsupported ABI."
#endif
@@ -151,7 +151,7 @@ extern __inline int
__attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
__crc_w_b_w (char _1, int _2)
{
- return (int) __builtin_loongarch_crc_w_b_w ((char) _1, (int) _2);
+ return __builtin_loongarch_crc_w_b_w (_1, _2);
}
/* Assembly instruction format: rd, rj, rk. */
@@ -160,7 +160,7 @@ extern __inline int
__attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
__crc_w_h_w (short _1, int _2)
{
- return (int) __builtin_loongarch_crc_w_h_w ((short) _1, (int) _2);
+ return __builtin_loongarch_crc_w_h_w (_1, _2);
}
/* Assembly instruction format: rd, rj, rk. */
@@ -169,7 +169,7 @@ extern __inline int
__attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
__crc_w_w_w (int _1, int _2)
{
- return (int) __builtin_loongarch_crc_w_w_w ((int) _1, (int) _2);
+ return __builtin_loongarch_crc_w_w_w (_1, _2);
}
#ifdef __loongarch64
@@ -179,7 +179,7 @@ extern __inline int
__attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
__crc_w_d_w (long int _1,
Re: [PATCH] x86-64: Generate push2/pop2 only if the incoming stack is 16-byte aligned
On Wed, Feb 14, 2024 at 5:33 AM H.J. Lu wrote:
>
> Since push2/pop2 requires 16-byte stack alignment, don't generate them
> if the incoming stack isn't 16-byte aligned.
Ok.
>
> gcc/
>
> PR target/113912
> * config/i386/i386.cc (ix86_can_use_push2pop2): New.
> (ix86_pro_and_epilogue_can_use_push2pop2): Use it.
> (ix86_emit_save_regs): Don't generate push2 if
> ix86_can_use_push2pop2 return false.
> (ix86_expand_epilogue): Don't generate pop2 if
> ix86_can_use_push2pop2 return false.
>
> gcc/testsuite/
>
> PR target/113912
> * gcc.target/i386/apx-push2pop2-2.c: New test.
> ---
> gcc/config/i386/i386.cc | 24 ++-
> .../gcc.target/i386/apx-push2pop2-2.c | 24 +++
> 2 files changed, 42 insertions(+), 6 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/i386/apx-push2pop2-2.c
>
> diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc
> index a4e12602f70..46f238651a6 100644
> --- a/gcc/config/i386/i386.cc
> +++ b/gcc/config/i386/i386.cc
> @@ -6802,16 +6802,24 @@ get_probe_interval (void)
>
> #define SPLIT_STACK_AVAILABLE 256
>
> -/* Helper function to determine whether push2/pop2 can be used in prologue or
> - epilogue for register save/restore. */
> +/* Return true if push2/pop2 can be generated. */
> +
> static bool
> -ix86_pro_and_epilogue_can_use_push2pop2 (int nregs)
> +ix86_can_use_push2pop2 (void)
> {
>/* Use push2/pop2 only if the incoming stack is 16-byte aligned. */
>unsigned int incoming_stack_boundary
> = (crtl->parm_stack_boundary > ix86_incoming_stack_boundary
> ? crtl->parm_stack_boundary : ix86_incoming_stack_boundary);
> - if (incoming_stack_boundary % 128 != 0)
> + return incoming_stack_boundary % 128 == 0;
> +}
> +
> +/* Helper function to determine whether push2/pop2 can be used in prologue or
> + epilogue for register save/restore. */
> +static bool
> +ix86_pro_and_epilogue_can_use_push2pop2 (int nregs)
> +{
> + if (!ix86_can_use_push2pop2 ())
> return false;
>int aligned = cfun->machine->fs.sp_offset % 16 == 0;
>return TARGET_APX_PUSH2POP2
> @@ -7401,7 +7409,9 @@ ix86_emit_save_regs (void)
>int regno;
>rtx_insn *insn;
>
> - if (!TARGET_APX_PUSH2POP2 || cfun->machine->func_type != TYPE_NORMAL)
> + if (!TARGET_APX_PUSH2POP2
> + || !ix86_can_use_push2pop2 ()
> + || cfun->machine->func_type != TYPE_NORMAL)
> {
>for (regno = FIRST_PSEUDO_REGISTER - 1; regno >= 0; regno--)
> if (GENERAL_REGNO_P (regno) && ix86_save_reg (regno, true, true))
> @@ -10039,7 +10049,9 @@ ix86_expand_epilogue (int style)
> m->fs.cfa_reg == stack_pointer_rtx);
> }
>
> - if (TARGET_APX_PUSH2POP2 && m->func_type == TYPE_NORMAL)
> + if (TARGET_APX_PUSH2POP2
> + && ix86_can_use_push2pop2 ()
> + && m->func_type == TYPE_NORMAL)
> ix86_emit_restore_regs_using_pop2 ();
>else
> ix86_emit_restore_regs_using_pop (TARGET_APX_PPX);
> diff --git a/gcc/testsuite/gcc.target/i386/apx-push2pop2-2.c
> b/gcc/testsuite/gcc.target/i386/apx-push2pop2-2.c
> new file mode 100644
> index 000..975a6212b30
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/apx-push2pop2-2.c
> @@ -0,0 +1,24 @@
> +/* { dg-do compile { target { ! ia32 } } } */
> +/* { dg-options "-O2 -mpreferred-stack-boundary=3 -mapx-features=push2pop2
> -fomit-frame-pointer" } */
> +
> +extern int bar (int);
> +
> +void foo ()
> +{
> + int a,b,c,d,e,f,i;
> + a = bar (5);
> + b = bar (a);
> + c = bar (b);
> + d = bar (c);
> + e = bar (d);
> + f = bar (e);
> + for (i = 1; i < 10; i++)
> + {
> +a += bar (a + i) + bar (b + i) +
> + bar (c + i) + bar (d + i) +
> + bar (e + i) + bar (f + i);
> + }
> +}
> +
> +/* { dg-final { scan-assembler-not "push2(|p)\[\\t \]*%r" } } */
> +/* { dg-final { scan-assembler-not "pop2(|p)\[\\t \]*%r" } } */
> --
> 2.43.0
>
--
BR,
Hongtao
Re: Re: [PATCH] RISC-V: Allow LICM hoist POLY_INT configuration code sequence
Hi, Robin. Could you continue on this LICM issue ?
I am not sure whether my fix is correct, or you may find another way to make
LICM works ?
juzhe.zh...@rivai.ai
From: Robin Dapp
Date: 2024-02-06 21:14
To: juzhe.zh...@rivai.ai; kito.cheng
CC: rdapp.gcc; gcc-patches; Kito.cheng; jeffreyalaw
Subject: Re: [PATCH] RISC-V: Allow LICM hoist POLY_INT configuration code
sequence
> The root cause is this following RTL pattern, after fwprop1:
>
> (insn 82 78 84 9 (set (reg:DI 230)
> (sign_extend:DI (minus:SI (subreg/s/v:SI (reg:DI 150 [ niters.10 ]) 0)
> (subreg:SI (reg:DI 221) 0 13 {subsi3_extended}
> (expr_list:REG_EQUAL (sign_extend:DI (plus:SI (subreg/s/v:SI (reg:DI 150
> [ niters.10 ]) 0)
> *(const_poly_int:SI [-16, -16])*))
> (nil)))
>
> The highlight *(const_poly_int:SI [-16, -16])*
> causes ICE.
>
> This RTL is because:
> (insn 69 68 71 8 (set (reg:DI 221)
> (const_poly_int:DI [16, 16])) 208 {*movdi_64bit}
> (nil))
> (insn 82 78 84 9 (set (reg:DI 230)
> (sign_extend:DI (minus:SI (subreg/s/v:SI (reg:DI 150 [ niters.10 ]) 0)
> (subreg:SI (reg:DI 221) 0 13 {subsi3_extended}
> > (subreg:SI (const_poly_int:SI [-16,
> -16])) fwprop1 add (const_poly_int:SI [-16, -16]) reg_equal
> (expr_list:REG_EQUAL (sign_extend:DI (plus:SI (subreg/s/v:SI (reg:DI 150
> [ niters.10 ]) 0)
> (const_poly_int:SI [-16, -16])))
> (nil)))
I'm seeing a slightly different pattern but that doesn't change
the problem.
> (set (reg:SI) (subreg:SI (DI: poly value))) but it causes ICE that I
> mentioned above.
That's indeed a bit more idiomatic and I wouldn't oppose that.
The problem causing the ICE is that we want to simplify a PLUS
with (const_poly_int:SI [16, 16]) and (const_int 0) but the mode
is DImode. My suspicion is that this is caused by our
addsi3_extended pattern and we fail to deduce the proper mode
for analysis.
I'm just speculating but maybe that's because we assert that a
plus is of the form simple_reg_p (op0) && CONSTANT_P (op1).
Usually, constants don't have a mode and can just be used.
poly_int_csts do have one and need to be explicitly converted
(kind of).
We can only analyze this zero_extended plus at all since Jeff
added the addsi3_extended handling for loop-iv. Maybe we could
punt like
diff --git a/gcc/loop-iv.cc b/gcc/loop-iv.cc
index eb7e923a38b..796413c25a3 100644
--- a/gcc/loop-iv.cc
+++ b/gcc/loop-iv.cc
@@ -714,6 +714,9 @@ get_biv_step_1 (df_ref def, scalar_int_mode outer_mode, rtx
reg,
if (!simple_reg_p (op0) || !CONSTANT_P (op1))
return false;
+ if (CONST_POLY_INT_P (op1) && GET_MODE (op1) != outer_mode)
+ return false;
+
This helps for your test case but I haven't done any further
testing. I'd think this is relatively safe because it's only
a missed analysis/optimization in the worst case.
Still, generally, I don't see a reason why we wouldn't be able
to analyze this?
Regards
Robin
