Re: [PATCH] libstdc++: Fix aliasing violation in std::shared_ptr
Thanks Jonathan! On Sun, Jan 23, 2022 at 5:51 PM Jonathan Wakely wrote: > I thought I'd CC'd Maged on this patch, but apparently not. I've > pushed it to trunk now. > > On Fri, 21 Jan 2022 at 13:50, Jonathan Wakely wrote: > > > > Tested powerpc64le-linux. Does anybody see a problem with this change? > > > > > > The non-atomic store that sets both reference counts to zero uses a > > type-punned pointer, which has undefined behaviour. We could use memset > > to write 8 bytes, but we don't actually need it to be a single store > > anyway. No other thread can observe the values, that's why it's safe to > > use non-atomic stores in the first place. So we can just set each count > > to zero. > > > > With -fstore-merging (which is enabled by default at -O2) GCC produces > > the same code for this as for memset or the type punned store. Clang > > does that store merging even at -O1. > > > > libstdc++-v3/ChangeLog: > > > > * include/bits/shared_ptr_base.h > (_Sp_counted_base<>::_M_release): > > Set members to zero without type punning. > > --- > > libstdc++-v3/include/bits/shared_ptr_base.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libstdc++-v3/include/bits/shared_ptr_base.h > b/libstdc++-v3/include/bits/shared_ptr_base.h > > index 5b8f84b65be..b2f955b41f7 100644 > > --- a/libstdc++-v3/include/bits/shared_ptr_base.h > > +++ b/libstdc++-v3/include/bits/shared_ptr_base.h > > @@ -340,7 +340,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > // we are releasing the last strong reference. No other > > // threads can observe the effects of this _M_release() > > // call (e.g. calling use_count()) without a data race. > > - *(long long*)(&_M_use_count) = 0; > > + _M_weak_count = _M_use_count = 0; > > _GLIBCXX_SYNCHRONIZATION_HAPPENS_AFTER(&_M_use_count); > > _GLIBCXX_SYNCHRONIZATION_HAPPENS_AFTER(&_M_weak_count); > > _M_dispose(); > > -- > > 2.31.1 > > > >
Re: [PATCH] RISC-V: Update testcases info with new implement info
Committed, thanks! On Wed, Jan 19, 2022 at 5:18 PM Martin Liška wrote: > > On 1/19/22 10:15, shi...@iscas.ac.cn wrote: > > |From: LiaoShihua After commit > > 591b6e00d1bfe12932ca31530d5859f95db8a35a " riscv: fix -Wformat-diag errors > > ", some strings in implement was changed. This patch update the check info > > in testcases to sync with it.| > > Thank you for the fix! > > Martin
Re: [PING^3][PATCH,v2,1/1,AARCH64][PR102768] aarch64: Add compiler support for Shadow Call Stack
On 1/20/22 04:02, Richard Sandiford wrote: Thanks for the patch and sorry for the (very) slow review. Thanks for the review, Richard :). +/* Handle a "no_sanitize_shadow_call_stack" attribute; arguments as in + struct attribute_spec.handler. */ +static tree +handle_no_sanitize_shadow_call_stack_attribute (tree *node, tree name, + tree, int, bool *no_add_attrs) +{ + *no_add_attrs = true; + if (TREE_CODE (*node) != FUNCTION_DECL) +warning (OPT_Wattributes, "%qE attribute ignored", name); + else +add_no_sanitize_value (*node, SANITIZE_SHADOW_CALL_STACK); + + return NULL_TREE; +} + Do we need this? I think these days the preference is to use the general "no_sanitize" attribute with an argument, which is also the syntax documented on the clang page. We have to support no_sanitize_foo attributes for some of the early sanitiser features, to avoid breaking backwards compatibility, but it doesn't look like clang ever supported no_sanitize_shadow_call_stack. Oh, "no_sanitize" is enough, I will delete this in the next version. +/* Return TRUE if shadow call stack should be enabled for the current + function, otherwise return FALSE. */ + +bool +aarch64_shadow_call_stack_enabled (void) +{ + /* This function should only be called after frame laid out. */ + gcc_assert (cfun->machine->frame.laid_out); + + if (crtl->calls_eh_return) +return false; + + /* We only deal with a function if its LR is pushed onto stack + and attribute no_sanitize_shadow_call_stack is not specified. */ (This would need to be updated if we do drop the specific attribute.) Ok. + /* Push return address to shadow call stack. */ + if (aarch64_shadow_call_stack_enabled ()) + emit_insn (gen_scs_push ()); Formatting nit: should only be indented by two spaces more than the “if”. Same below. Got it. I guess doing it like this means that we also continue to store x30 to the frame in the traditional way. And that's probably necessary, given that the saved x30 forms part of link chain. Yes, we just added an extra insn to the prologue like: + str x30, [x18], #8 stp x29, x30, [sp, #-16]! ... + if (flag_stack_usage_info) current_function_static_stack_size = constant_lower_bound (frame_size); @@ -9066,6 +9089,10 @@ aarch64_expand_epilogue (bool for_sibcall) RTX_FRAME_RELATED_P (insn) = 1; } + /* Pop return address from shadow call stack. */ + if (aarch64_shadow_call_stack_enabled ()) + emit_insn (gen_scs_pop ()); + This looks correct, but following on from the above, I guess we continue to restore x30 from the frame in the traditional way first, and then overwrite it with the SCS value. I think the output code would be slightly neater if we suppressed the first restore of x30. Yes, the current epilogue is like: ... ldp x29, x30, [sp], #16 + ldr x30, [x18, #-8]! I think may be we can keep the two x30 pops here, for two reasons: 1) The implementation of scs in clang is to pop x30 twice, it might be better to be consistent with clang here[1]. 2) If we keep the first restore of x30, it may provide some flexibility for other scenarios. For example, we can directly patch the scs_push/pop insns in the binary to turn it into a binary without scs; [1] https://github.com/llvm/llvm-project/commit/f11eb3ebe77729426e562d7d4d7ebb1d5ff2e7c8 +/* This value represents whether the shadow call stack is implemented on + the target platform. */ +#define TARGET_SUPPORT_SHADOW_CALL_STACK true + +#define TARGET_CHECK_SCS_RESERVED_REGISTER() \ + do { \ +if (!fixed_regs[R18_REGNUM]) \ + error ("%<-fsanitize=shadow-call-stack%> only " \ +"allowed with %<-ffixed-x18%>"); \ + } while (0) + Please make these target hooks instead. The first one can use DEFHOOKPOD. Ok, I will add a field to gcc_target via DEFHOOKPOD. +;; Save X30 in the X18-based POST_INC stack (consistent with clang). +(define_insn "scs_push" + [(set (mem:DI (reg:DI R18_REGNUM)) (reg:DI R30_REGNUM)) + (set (reg:DI R18_REGNUM) (plus:DI (reg:DI R18_REGNUM) (const_int 8)))] + "" + "str\\tx30, [x18], #8" + [(set_attr "type" "store_8")] +) + +;; Load X30 form the X18-based PRE_DEC stack (consistent with clang). +(define_insn "scs_pop" + [(set (reg:DI R18_REGNUM) (minus:DI (reg:DI R18_REGNUM) (const_int 8))) + (set (reg:DI R30_REGNUM) (mem:DI (reg:DI R18_REGNUM)))] + "" + "ldr\\tx30, [x18, #-8]!" + [(set_attr "type" "load_8")] +) + The two SETs here work in parallel, with the define_insn as a whole following a "read input operands, act, write output operands" sequence. The (mem: …) address therefore sees the old value of r18 rather than the new one. Also, RTL rules say that subtractions need to be written as additions of the negative. Oh, sorry, I got it wrong here. I think the pattern would therefore be something l
Re: [PATCH v2] x86: Also check mode of memory broadcast in bcst_mem_operand
On Sun, Jan 23, 2022 at 4:35 PM Hongtao Liu wrote: > > On Sun, Jan 23, 2022 at 8:28 PM H.J. Lu via Gcc-patches > wrote: > > > > Return false for invalid mode on memory broadcast in bcst_mem_operand: > > > > (vec_duplicate:V16SF (mem/j:V4SF (reg/v/f:DI 109 [ b ]))) > > > Yes, thanks. I will also backport it to GCC 11 branch. Thanks. > > gcc/ > > > > PR target/104188 > > * config/i386/predicates.md (bcst_mem_operand): Also check mode > > of memory broadcast. > > > > gcc/testsuite/ > > > > PR target/104188 > > * gcc.target/i386/pr104188.c: New test. > > --- > > gcc/config/i386/predicates.md| 2 + > > gcc/testsuite/gcc.target/i386/pr104188.c | 70 > > 2 files changed, 72 insertions(+) > > create mode 100644 gcc/testsuite/gcc.target/i386/pr104188.c > > > > diff --git a/gcc/config/i386/predicates.md b/gcc/config/i386/predicates.md > > index eae6ab58e23..a8cc17a054d 100644 > > --- a/gcc/config/i386/predicates.md > > +++ b/gcc/config/i386/predicates.md > > @@ -1157,6 +1157,8 @@ (define_predicate "bcst_mem_operand" > > (ior (match_test "TARGET_AVX512VL") > > (match_test "GET_MODE_SIZE (GET_MODE (op)) == 64"))) > > (match_test "VALID_BCST_MODE_P (GET_MODE_INNER (GET_MODE (op)))") > > + (match_test "GET_MODE (XEXP (op, 0)) > > + == GET_MODE_INNER (GET_MODE (op))") > > (match_test "memory_operand (XEXP (op, 0), GET_MODE (XEXP (op, > > 0)))"))) > > > > ; Return true when OP is bcst_mem_operand or vector_memory_operand. > > diff --git a/gcc/testsuite/gcc.target/i386/pr104188.c > > b/gcc/testsuite/gcc.target/i386/pr104188.c > > new file mode 100644 > > index 000..c6f615b9625 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.target/i386/pr104188.c > > @@ -0,0 +1,70 @@ > > +/* { dg-do run { target avx512f } } */ > > +/* { dg-options "-O2 -mfpmath=sse" } */ > > + > > +#include > > + > > +union U { > > + float m[4][4]; > > + __m128 r[4]; > > + __m512 s; > > +}; > > + > > +__attribute__((noipa, target("avx512f"))) > > +void > > +foo (union U *x, union U *a, union U *b) > > +{ > > + __m512 c = _mm512_loadu_ps (&a->s); > > + __m512 d = _mm512_broadcast_f32x4 (b->r[0]); > > + __m512 e = _mm512_broadcast_f32x4 (b->r[1]); > > + __m512 f = _mm512_broadcast_f32x4 (b->r[2]); > > + __m512 g = _mm512_broadcast_f32x4 (b->r[3]); > > + __m512 h = _mm512_mul_ps (_mm512_permute_ps (c, 0x00), d); > > + h = _mm512_fmadd_ps (_mm512_permute_ps (c, 0x55), e, h); > > + h = _mm512_fmadd_ps (_mm512_permute_ps (c, 0xaa), f, h); > > + h = _mm512_fmadd_ps (_mm512_permute_ps (c, 0xff), g, h); > > + _mm512_storeu_ps (&x->s, h); > > +} > > + > > +__attribute__((noipa, target("avx512f"))) > > +void > > +do_test (void) > > +{ > > + union U a = { .m = { { 1.0f, 2.0f, 3.0f, 4.0f }, > > + { 5.0f, 6.0f, 7.0f, 8.0f }, > > + { 9.0f, 10.0f, 11.0f, 12.0f }, > > + { 13.0f, 14.0f, 15.0f, 16.0f } } }; > > + union U b = { .m = { { 17.0f, 18.0f, 19.0f, 20.0f }, > > + { 21.0f, 22.0f, 23.0f, 24.0f }, > > + { 25.0f, 26.0f, 27.0f, 28.0f }, > > + { 29.0f, 30.0f, 31.0f, 32.0f } } }; > > + union U c; > > + foo (&c, &a, &b); > > + if (c.m[0][0] != 250.0f > > + || c.m[0][1] != 260.0f > > + || c.m[0][2] != 270.0f > > + || c.m[0][3] != 280.0f) > > +__builtin_abort (); > > + if (c.m[1][0] != 618.0f > > + || c.m[1][1] != 644.0f > > + || c.m[1][2] != 670.0f > > + || c.m[1][3] != 696.0f) > > +__builtin_abort (); > > + if (c.m[2][0] != 986.0f > > + || c.m[2][1] != 1028.0f > > + || c.m[2][2] != 1070.0f > > + || c.m[2][3] != 1112.0f) > > +__builtin_abort (); > > + if (c.m[3][0] != 1354.0f > > + || c.m[3][1] != 1412.0f > > + || c.m[3][2] != 1470.0f > > + || c.m[3][3] != 1528.0f) > > +__builtin_abort (); > > +} > > + > > +int > > +main () > > +{ > > + if (__builtin_cpu_supports ("avx512f")) > > +do_test (); > > + return 0; > > +} > > -- > > 2.34.1 > > > > > -- > BR, > Hongtao -- H.J.
Re: [PATCH v2] x86: Also check mode of memory broadcast in bcst_mem_operand
On Sun, Jan 23, 2022 at 8:28 PM H.J. Lu via Gcc-patches wrote: > > Return false for invalid mode on memory broadcast in bcst_mem_operand: > > (vec_duplicate:V16SF (mem/j:V4SF (reg/v/f:DI 109 [ b ]))) > Yes, thanks. > gcc/ > > PR target/104188 > * config/i386/predicates.md (bcst_mem_operand): Also check mode > of memory broadcast. > > gcc/testsuite/ > > PR target/104188 > * gcc.target/i386/pr104188.c: New test. > --- > gcc/config/i386/predicates.md| 2 + > gcc/testsuite/gcc.target/i386/pr104188.c | 70 > 2 files changed, 72 insertions(+) > create mode 100644 gcc/testsuite/gcc.target/i386/pr104188.c > > diff --git a/gcc/config/i386/predicates.md b/gcc/config/i386/predicates.md > index eae6ab58e23..a8cc17a054d 100644 > --- a/gcc/config/i386/predicates.md > +++ b/gcc/config/i386/predicates.md > @@ -1157,6 +1157,8 @@ (define_predicate "bcst_mem_operand" > (ior (match_test "TARGET_AVX512VL") > (match_test "GET_MODE_SIZE (GET_MODE (op)) == 64"))) > (match_test "VALID_BCST_MODE_P (GET_MODE_INNER (GET_MODE (op)))") > + (match_test "GET_MODE (XEXP (op, 0)) > + == GET_MODE_INNER (GET_MODE (op))") > (match_test "memory_operand (XEXP (op, 0), GET_MODE (XEXP (op, > 0)))"))) > > ; Return true when OP is bcst_mem_operand or vector_memory_operand. > diff --git a/gcc/testsuite/gcc.target/i386/pr104188.c > b/gcc/testsuite/gcc.target/i386/pr104188.c > new file mode 100644 > index 000..c6f615b9625 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/pr104188.c > @@ -0,0 +1,70 @@ > +/* { dg-do run { target avx512f } } */ > +/* { dg-options "-O2 -mfpmath=sse" } */ > + > +#include > + > +union U { > + float m[4][4]; > + __m128 r[4]; > + __m512 s; > +}; > + > +__attribute__((noipa, target("avx512f"))) > +void > +foo (union U *x, union U *a, union U *b) > +{ > + __m512 c = _mm512_loadu_ps (&a->s); > + __m512 d = _mm512_broadcast_f32x4 (b->r[0]); > + __m512 e = _mm512_broadcast_f32x4 (b->r[1]); > + __m512 f = _mm512_broadcast_f32x4 (b->r[2]); > + __m512 g = _mm512_broadcast_f32x4 (b->r[3]); > + __m512 h = _mm512_mul_ps (_mm512_permute_ps (c, 0x00), d); > + h = _mm512_fmadd_ps (_mm512_permute_ps (c, 0x55), e, h); > + h = _mm512_fmadd_ps (_mm512_permute_ps (c, 0xaa), f, h); > + h = _mm512_fmadd_ps (_mm512_permute_ps (c, 0xff), g, h); > + _mm512_storeu_ps (&x->s, h); > +} > + > +__attribute__((noipa, target("avx512f"))) > +void > +do_test (void) > +{ > + union U a = { .m = { { 1.0f, 2.0f, 3.0f, 4.0f }, > + { 5.0f, 6.0f, 7.0f, 8.0f }, > + { 9.0f, 10.0f, 11.0f, 12.0f }, > + { 13.0f, 14.0f, 15.0f, 16.0f } } }; > + union U b = { .m = { { 17.0f, 18.0f, 19.0f, 20.0f }, > + { 21.0f, 22.0f, 23.0f, 24.0f }, > + { 25.0f, 26.0f, 27.0f, 28.0f }, > + { 29.0f, 30.0f, 31.0f, 32.0f } } }; > + union U c; > + foo (&c, &a, &b); > + if (c.m[0][0] != 250.0f > + || c.m[0][1] != 260.0f > + || c.m[0][2] != 270.0f > + || c.m[0][3] != 280.0f) > +__builtin_abort (); > + if (c.m[1][0] != 618.0f > + || c.m[1][1] != 644.0f > + || c.m[1][2] != 670.0f > + || c.m[1][3] != 696.0f) > +__builtin_abort (); > + if (c.m[2][0] != 986.0f > + || c.m[2][1] != 1028.0f > + || c.m[2][2] != 1070.0f > + || c.m[2][3] != 1112.0f) > +__builtin_abort (); > + if (c.m[3][0] != 1354.0f > + || c.m[3][1] != 1412.0f > + || c.m[3][2] != 1470.0f > + || c.m[3][3] != 1528.0f) > +__builtin_abort (); > +} > + > +int > +main () > +{ > + if (__builtin_cpu_supports ("avx512f")) > +do_test (); > + return 0; > +} > -- > 2.34.1 > -- BR, Hongtao
Re: [PATCH] libstdc++: Fix aliasing violation in std::shared_ptr
I thought I'd CC'd Maged on this patch, but apparently not. I've pushed it to trunk now. On Fri, 21 Jan 2022 at 13:50, Jonathan Wakely wrote: > > Tested powerpc64le-linux. Does anybody see a problem with this change? > > > The non-atomic store that sets both reference counts to zero uses a > type-punned pointer, which has undefined behaviour. We could use memset > to write 8 bytes, but we don't actually need it to be a single store > anyway. No other thread can observe the values, that's why it's safe to > use non-atomic stores in the first place. So we can just set each count > to zero. > > With -fstore-merging (which is enabled by default at -O2) GCC produces > the same code for this as for memset or the type punned store. Clang > does that store merging even at -O1. > > libstdc++-v3/ChangeLog: > > * include/bits/shared_ptr_base.h (_Sp_counted_base<>::_M_release): > Set members to zero without type punning. > --- > libstdc++-v3/include/bits/shared_ptr_base.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libstdc++-v3/include/bits/shared_ptr_base.h > b/libstdc++-v3/include/bits/shared_ptr_base.h > index 5b8f84b65be..b2f955b41f7 100644 > --- a/libstdc++-v3/include/bits/shared_ptr_base.h > +++ b/libstdc++-v3/include/bits/shared_ptr_base.h > @@ -340,7 +340,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > // we are releasing the last strong reference. No other > // threads can observe the effects of this _M_release() > // call (e.g. calling use_count()) without a data race. > - *(long long*)(&_M_use_count) = 0; > + _M_weak_count = _M_use_count = 0; > _GLIBCXX_SYNCHRONIZATION_HAPPENS_AFTER(&_M_use_count); > _GLIBCXX_SYNCHRONIZATION_HAPPENS_AFTER(&_M_weak_count); > _M_dispose(); > -- > 2.31.1 >
[committed] libstdc++: Fix std::spanstream move assignment [PR104032]
Tested powerpc64le-linux, pushed to trunk. libstdc++-v3/ChangeLog: PR libstdc++/104032 * include/std/spanstream (basic_spanbuf(basic_spanbuf&&)): Use mem-initializer for _M_buf. (basic_spanbuf::Operator=(basic_spanbuf&&)): Fix ill-formed member access. * testsuite/27_io/spanstream/2.cc: New test. --- libstdc++-v3/include/std/spanstream | 15 ++- libstdc++-v3/testsuite/27_io/spanstream/2.cc | 113 +++ 2 files changed, 124 insertions(+), 4 deletions(-) create mode 100644 libstdc++-v3/testsuite/27_io/spanstream/2.cc diff --git a/libstdc++-v3/include/std/spanstream b/libstdc++-v3/include/std/spanstream index 240866ff26f..000bda52a1e 100644 --- a/libstdc++-v3/include/std/spanstream +++ b/libstdc++-v3/include/std/spanstream @@ -75,10 +75,17 @@ template> basic_spanbuf(const basic_spanbuf&) = delete; -/// Move constructor. In this implementation `rhs` is left unchanged. +/** Move constructor. + * + * Transfers the buffer and pointers into the get and put areas from + * `__rhs` to `*this`. + * + * In this implementation `rhs` is left unchanged, + * but that is not guaranteed by the standard. + */ basic_spanbuf(basic_spanbuf&& __rhs) -: __streambuf_type(__rhs), _M_mode(__rhs._M_mode) -{ span(__rhs._M_buf); } +: __streambuf_type(__rhs), _M_mode(__rhs._M_mode), _M_buf(__rhs._M_buf) +{ } // [spanbuf.assign], assignment and swap basic_spanbuf& operator=(const basic_spanbuf&) = delete; @@ -86,7 +93,7 @@ template> basic_spanbuf& operator=(basic_spanbuf&& __rhs) { - basic_spanbuf(std::move(__rhs))->swap(*this); + basic_spanbuf(std::move(__rhs)).swap(*this); return *this; } diff --git a/libstdc++-v3/testsuite/27_io/spanstream/2.cc b/libstdc++-v3/testsuite/27_io/spanstream/2.cc new file mode 100644 index 000..a13a50b0dce --- /dev/null +++ b/libstdc++-v3/testsuite/27_io/spanstream/2.cc @@ -0,0 +1,113 @@ +// { dg-options "-std=gnu++23" } +// { dg-do run { target c++23 } } + +#include +#include + +using std::ispanstream; +using std::ospanstream; +using std::span; + +void +test_move() +{ + char c; + { +const char str[] = "chars"; +std::ispanstream a(str); +std::ispanstream b = std::move(a); +VERIFY( b.span().data() == str && b.span().size() == 6 ); +VERIFY( b >> c ); +VERIFY( c == 'c' ); + +a = std::move(b); +VERIFY( a.span().data() == str && a.span().size() == 6 ); +VERIFY( a >> c >> c ); +VERIFY( c == 'a' ); + } + + { +char buf[10] = {}; +std::ospanstream a(buf); +std::ospanstream b = std::move(a); +VERIFY( b << 'c' ); +VERIFY( buf[0] == 'c' ); +VERIFY( !std::char_traits::compare(buf, "c", 2) ); + +a = std::move(b); +VERIFY( a << 'h' << 'a' << "rs" ); +VERIFY( !std::char_traits::compare(buf, "chars", 6) ); + } + + { +char buf[10] = {}; +std::spanstream a(buf); +std::spanstream b = std::move(a); +VERIFY( b.span().empty() ); +VERIFY( b << 'c' ); +VERIFY( buf[0] == 'c' ); +VERIFY( !std::char_traits::compare(buf, "c", 2) ); +VERIFY( b.span().data() == buf && b.span().size() == 1 ); +VERIFY( b >> c ); +VERIFY( c == 'c' ); + +a = std::move(b); +VERIFY( a.span().data() == buf && a.span().size() == 1 ); +VERIFY( a << 'h' << 'a' << "rs" ); +VERIFY( !std::char_traits::compare(buf, "chars", 6) ); +VERIFY( a.span().data() == buf && a.span().size() == 5 ); + } +} + +void +test_swap() +{ + { +const char str1[] = "chars"; +const char str2[] = "STRING"; +std::ispanstream a(str1); +std::ispanstream b(str2); +a.swap(b); +VERIFY( a.span().data() == str2 && a.span().size() == 7 ); +VERIFY( b.span().data() == str1 && b.span().size() == 6 ); +char c; +VERIFY( a >> c ); +VERIFY( c == 'S' ); +VERIFY( b >> c ); +VERIFY( c == 'c' ); + +swap(a, b); +VERIFY( a.span().data() == str1 && a.span().size() == 6 ); +VERIFY( b.span().data() == str2 && b.span().size() == 7 ); +VERIFY( a >> c >> c ); +VERIFY( c == 'a' ); +VERIFY( b >> c >> c ); +VERIFY( c == 'R' ); + } + + { +char buf1[] = "xxx"; +char buf2[] = "xxx"; +std::ospanstream a(buf1); +std::ospanstream b(buf2); +a.swap(b); +VERIFY( a << "STR" ); +VERIFY( !std::char_traits::compare(buf2, "STRx", 4) ); +VERIFY( b << 'c' << 'h' ); +VERIFY( !std::char_traits::compare(buf1, "chx", 3) ); + +swap(a, b); +VERIFY( a.span().size() == 2 ); +VERIFY( b.span().size() == 3 ); +VERIFY( a << 'a' << "rs" ); +VERIFY( !std::char_traits::compare(buf1, "charsx", 6) ); +VERIFY( b << "IN" << 'G' ); +VERIFY( !std::char_traits::compare(buf2, "STRINGx", 7) ); + } +} + +int main() +{ + test_move(); + test_swap(); +} -- 2.31.1
[committed] libstdc++: Use fast_float for long double if it uses binary64 format
Tested powerpc64le-linux, pushed to trunk. We can use the new from_chars implementation when long double and double have the same representation. libstdc++-v3/ChangeLog: * src/c++17/floating_from_chars.cc (USE_STRTOD_FOR_FROM_CHARS): Define macro for case where std::from_chars is implemented in terms of strtod, strtof or strtold. (buffer_resource, valid_fmt, find_end_of_float, pattern) (from_chars_impl, make_result, reserve_string): Do not define unless USE_STRTOD_FOR_FROM_CHARS is defined. (from_chars): Define when at least one of USE_LIB_FAST_FLOAT and USE_STRTOD_FOR_FROM_CHARS is defined, instead of _GLIBCXX_HAVE_USELOCALE. Use fast_float for long double when it is binary64. --- libstdc++-v3/src/c++17/floating_from_chars.cc | 38 --- 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/libstdc++-v3/src/c++17/floating_from_chars.cc b/libstdc++-v3/src/c++17/floating_from_chars.cc index 3158a3a96d3..ba1345db3f2 100644 --- a/libstdc++-v3/src/c++17/floating_from_chars.cc +++ b/libstdc++-v3/src/c++17/floating_from_chars.cc @@ -46,6 +46,13 @@ # include #endif +#if _GLIBCXX_HAVE_USELOCALE +// FIXME: This should be reimplemented so it doesn't use strtod and newlocale. +// That will avoid the need for any memory allocation, meaning that the +// non-conforming errc::not_enough_memory result cannot happen. +# define USE_STRTOD_FOR_FROM_CHARS 1 +#endif + #ifdef _GLIBCXX_LONG_DOUBLE_ALT128_COMPAT #ifndef __LONG_DOUBLE_IBM128__ #error "floating_from_chars.cc must be compiled with -mabi=ibmlongdouble" @@ -56,6 +63,9 @@ extern "C" __ieee128 __strtoieee128(const char*, char**); #if _GLIBCXX_FLOAT_IS_IEEE_BINARY32 && _GLIBCXX_DOUBLE_IS_IEEE_BINARY64 # define USE_LIB_FAST_FLOAT 1 +# if __LDBL_MANT_DIG__ == __DBL_MANT_DIG__ +# undef USE_STRTOD_FOR_FROM_CHARS +# endif #endif #if USE_LIB_FAST_FLOAT @@ -66,13 +76,13 @@ namespace } // anon namespace #endif -#if _GLIBCXX_HAVE_USELOCALE namespace std _GLIBCXX_VISIBILITY(default) { _GLIBCXX_BEGIN_NAMESPACE_VERSION namespace { +#if USE_STRTOD_FOR_FROM_CHARS // A memory resource with a static buffer that can be used for small // allocations. At most one allocation using the freestore can be done // if the static buffer is insufficient. The callers below only require @@ -409,6 +419,7 @@ namespace return true; } #endif +#endif // USE_STRTOD_FOR_FROM_CHARS #if _GLIBCXX_FLOAT_IS_IEEE_BINARY32 && _GLIBCXX_DOUBLE_IS_IEEE_BINARY64 // If the given ASCII character represents a hexit, return that hexit. @@ -771,13 +782,11 @@ namespace return {first, errc{}}; } -#endif +#endif // _GLIBCXX_FLOAT_IS_IEEE_BINARY32 && _GLIBCXX_DOUBLE_IS_IEEE_BINARY64 } // namespace -// FIXME: This should be reimplemented so it doesn't use strtod and newlocale. -// That will avoid the need for any memory allocation, meaning that the -// non-conforming errc::not_enough_memory result cannot happen. +#if USE_LIB_FAST_FLOAT || USE_STRTOD_FOR_FROM_CHARS from_chars_result from_chars(const char* first, const char* last, float& value, @@ -855,6 +864,21 @@ from_chars_result from_chars(const char* first, const char* last, long double& value, chars_format fmt) noexcept { +#if _GLIBCXX_FLOAT_IS_IEEE_BINARY32 && _GLIBCXX_DOUBLE_IS_IEEE_BINARY64 \ + && ! USE_STRTOD_FOR_FROM_CHARS + double dbl_value; + from_chars_result result; + if (fmt == chars_format::hex) +result = __floating_from_chars_hex(first, last, dbl_value); + else +{ + static_assert(USE_LIB_FAST_FLOAT); + result = fast_float::from_chars(first, last, dbl_value, fmt); +} + if (result.ec == errc{}) +value = dbl_value; + return result; +#else errc ec = errc::invalid_argument; #if _GLIBCXX_USE_CXX11_ABI buffer_resource mr; @@ -875,6 +899,7 @@ from_chars(const char* first, const char* last, long double& value, fmt = chars_format{}; } return make_result(first, len, fmt, ec); +#endif } #ifdef _GLIBCXX_LONG_DOUBLE_COMPAT @@ -909,6 +934,7 @@ from_chars(const char* first, const char* last, __ieee128& value, } #endif +#endif // USE_LIB_FAST_FLOAT || USE_STRTOD_FOR_FROM_CHARS + _GLIBCXX_END_NAMESPACE_VERSION } // namespace std -#endif // _GLIBCXX_HAVE_USELOCALE -- 2.31.1
[committed] libstdc++: Restore support for unordered_map [PR104174]
Tested powerpc64le-linux, pushed to trunk. I broke this unintentionally in r12-4259. libstdc++-v3/ChangeLog: PR libstdc++/104174 * include/bits/hashtable_policy.h (_Map_base): Add partial specialization for maps with const key types. * testsuite/23_containers/unordered_map/104174.cc: New test. --- libstdc++-v3/include/bits/hashtable_policy.h | 11 +++ .../testsuite/23_containers/unordered_map/104174.cc | 4 2 files changed, 15 insertions(+) create mode 100644 libstdc++-v3/testsuite/23_containers/unordered_map/104174.cc diff --git a/libstdc++-v3/include/bits/hashtable_policy.h b/libstdc++-v3/include/bits/hashtable_policy.h index 3b60eb9ae72..0f0b0f9ea51 100644 --- a/libstdc++-v3/include/bits/hashtable_policy.h +++ b/libstdc++-v3/include/bits/hashtable_policy.h @@ -812,6 +812,17 @@ namespace __detail return __pos->second; } + // Partial specialization for unordered_map, see PR 104174. + template +struct _Map_base, +_Alloc, _Select1st, _Equal, _Hash, +_RangeHash, _Unused, _RehashPolicy, _Traits, __uniq> +: _Map_base<_Key, pair, _Alloc, _Select1st, _Equal, _Hash, + _RangeHash, _Unused, _RehashPolicy, _Traits, __uniq> +{ }; + /** * Primary class template _Insert_base. * diff --git a/libstdc++-v3/testsuite/23_containers/unordered_map/104174.cc b/libstdc++-v3/testsuite/23_containers/unordered_map/104174.cc new file mode 100644 index 000..4007425bf74 --- /dev/null +++ b/libstdc++-v3/testsuite/23_containers/unordered_map/104174.cc @@ -0,0 +1,4 @@ +// { dg-do compile { target c++11 } } +// PR libstdc++/104174 unordered_map fails +#include +std::unordered_map> m; -- 2.31.1
Re: [PATCH] c++: value category of compound object expr [PR104173]
On 1/22/22 16:24, Patrick Palka wrote: Here the call to the &&-qualified toLower() is incorrectly rejected because the object expression is deemed to be an lvalue, even though it's really a prvalue. The object expression, instance()->applicationName(), is expressed as an INDIRECT_REF of a COMPOUND_EXPR *(*instance ();, &TARGET_EXPR ;); which lvalue_kind categorizes as an lvalue. This issue seems similar to PR88103 except that here the compound object expression is a prvalue rather than an xvalue. The fix there was to adjust the result of unary_complex_lvalue in build_class_member_access_expr so that xvalue-ness is preserved. This patch extends the fix so that rvalue-ness is preserved more generally. Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for trunk? OK. PR c++/104173 gcc/cp/ChangeLog: * typeck.cc (build_class_member_access_expr): Extend unary_complex_lvalue result adjustment to preserve all rvalues, not just xvalues. gcc/testsuite/ChangeLog: * g++.dg/cpp0x/ref-qual21.C: New test. --- gcc/cp/typeck.cc| 19 --- gcc/testsuite/g++.dg/cpp0x/ref-qual21.C | 16 2 files changed, 24 insertions(+), 11 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp0x/ref-qual21.C diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc index 3a28d639cb1..59668203573 100644 --- a/gcc/cp/typeck.cc +++ b/gcc/cp/typeck.cc @@ -2726,17 +2726,14 @@ build_class_member_access_expr (cp_expr object, tree member, /* Transform `(a, b).x' into `(*(a, &b)).x', `(a ? b : c).x' into `(*(a ? &b : &c)).x', and so on. A COND_EXPR is only an lvalue in the front end; only _DECLs and _REFs are lvalues in the back end. */ - { -tree temp = unary_complex_lvalue (ADDR_EXPR, object); -if (temp) - { - temp = cp_build_fold_indirect_ref (temp); - if (xvalue_p (object) && !xvalue_p (temp)) - /* Preserve xvalue kind. */ - temp = move (temp); - object = temp; - } - } + if (tree temp = unary_complex_lvalue (ADDR_EXPR, object)) +{ + temp = cp_build_fold_indirect_ref (temp); + if (!lvalue_p (object) && lvalue_p (temp)) + /* Preserve rvalue-ness. */ + temp = move (temp); + object = temp; +} /* In [expr.ref], there is an explicit list of the valid choices for MEMBER. We check for each of those cases here. */ diff --git a/gcc/testsuite/g++.dg/cpp0x/ref-qual21.C b/gcc/testsuite/g++.dg/cpp0x/ref-qual21.C new file mode 100644 index 000..e2c43c345ab --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/ref-qual21.C @@ -0,0 +1,16 @@ +// PR c++/104173 +// { dg-do compile { target c++11 } } + +struct QString { + QString toLower() &&; +}; + +struct QCoreApplication { + static QString applicationName(); +}; + +QCoreApplication *instance(); + +int main() { + instance()->applicationName().toLower(); +}
Re: [PATCH v3] c++: designated init of char array by string constant [PR55227]
On 1/17/22 14:29, will wray wrote: Attached (the cut n paste looks like it removed some whitespace formatting) Pushed, thanks! I incorporated the introduction from your initial email in the commit message. Jason
[PATCH] [aarch64/64821]: Simplify __builtin_aarch64_sqrt* into internal function .SQRT.
From: Andrew Pinski This is a simple patch which simplifies the __builtin_aarch64_sqrt* builtins into the internal function SQRT which allows for constant folding and other optimizations at the gimple level. It was originally suggested we do to __builtin_sqrt just for __builtin_aarch64_sqrtdf when -fno-math-errno but since r6-4969-g686ee9719a4 we have the internal function SQRT which does the same so it makes we don't need to check -fno-math-errno either now. Applied as approved after bootstrapped and tested on aarch64-linux-gnu with no regressions. PR target/64821 gcc/ChangeLog: * config/aarch64/aarch64-builtins.cc (aarch64_general_gimple_fold_builtin): Handle __builtin_aarch64_sqrt* and simplify into SQRT internal function. gcc/testsuite/ChangeLog: * gcc.target/aarch64/vsqrt-1.c: New test. * gcc.target/aarch64/vsqrt-2.c: New test. --- gcc/config/aarch64/aarch64-builtins.cc | 7 ++ gcc/testsuite/gcc.target/aarch64/vsqrt-1.c | 17 + gcc/testsuite/gcc.target/aarch64/vsqrt-2.c | 28 ++ 3 files changed, 52 insertions(+) create mode 100644 gcc/testsuite/gcc.target/aarch64/vsqrt-1.c create mode 100644 gcc/testsuite/gcc.target/aarch64/vsqrt-2.c diff --git a/gcc/config/aarch64/aarch64-builtins.cc b/gcc/config/aarch64/aarch64-builtins.cc index b7f338d6229..5217dbdb2ac 100644 --- a/gcc/config/aarch64/aarch64-builtins.cc +++ b/gcc/config/aarch64/aarch64-builtins.cc @@ -2820,6 +2820,13 @@ aarch64_general_gimple_fold_builtin (unsigned int fcode, gcall *stmt, gimple_call_set_lhs (new_stmt, gimple_call_lhs (stmt)); break; + /* Lower sqrt builtins to gimple/internal function sqrt. */ + BUILTIN_VHSDF_DF (UNOP, sqrt, 2, FP) + new_stmt = gimple_build_call_internal (IFN_SQRT, + 1, args[0]); + gimple_call_set_lhs (new_stmt, gimple_call_lhs (stmt)); + break; + /*lower store and load neon builtins to gimple. */ BUILTIN_VALL_F16 (LOAD1, ld1, 0, LOAD) BUILTIN_VDQ_I (LOAD1_U, ld1, 0, LOAD) diff --git a/gcc/testsuite/gcc.target/aarch64/vsqrt-1.c b/gcc/testsuite/gcc.target/aarch64/vsqrt-1.c new file mode 100644 index 000..e614c7d5a0f --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/vsqrt-1.c @@ -0,0 +1,17 @@ +/* PR target/64821 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-optimized" } */ +/* Check that we constant fold sqrt(4.0) into 2.0. */ +/* { dg-final { scan-tree-dump-not " \\\.SQRT" "optimized" } } */ +/* { dg-final { scan-tree-dump " 2\\\.0e\\\+0" "optimized" } } */ +/* { dg-final { scan-assembler-not "fsqrt" } } */ +/* We should produce a fmov to d0 with 2.0 but currently don't, see PR 103959. */ +/* { dg-final { scan-assembler-times "\n\tfmov\td0, 2.0e.0" 1 { xfail *-*-* } } } */ + +#include + +float64x1_t f64(void) +{ + float64x1_t a = (float64x1_t){4.0}; + return vsqrt_f64 (a); +} diff --git a/gcc/testsuite/gcc.target/aarch64/vsqrt-2.c b/gcc/testsuite/gcc.target/aarch64/vsqrt-2.c new file mode 100644 index 000..4dea4da7da6 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/vsqrt-2.c @@ -0,0 +1,28 @@ +/* PR target/64821 */ +/* { dg-do compile } */ +/* { dg-options "-fdump-tree-optimized" } */ +#include + +/* Check that we lower __builtin_aarch64_sqrt* into the internal function SQRT. */ +/* { dg-final { scan-tree-dump-times " __builtin_aarch64_sqrt" 0 "optimized" } } */ +/* { dg-final { scan-tree-dump-times " \\\.SQRT " 4 "optimized" } } */ + +float64x1_t f64(float64x1_t a) +{ + return vsqrt_f64 (a); +} + +float64x2_t f64q(float64x2_t a) +{ + return vsqrtq_f64 (a); +} + +float32x2_t f32(float32x2_t a) +{ + return vsqrt_f32 (a); +} + +float32x4_t f32q(float32x4_t a) +{ + return vsqrtq_f32 (a); +} -- 2.17.1
Re: [PATCH] riscv: fix -Wformat-diag errors.
On Tue, 18 Jan 2022, Palmer Dabbelt wrote: > Yep. Seeing this go by, though, I think there's some English issues with the > original messages. I'd write it more like this, but I'm never 100% sure on > these things: > >diff --git a/gcc/common/config/riscv/riscv-common.cc > b/gcc/common/config/riscv/riscv-common.cc >index 004822bfe6c..2f83303ca51 100644 >--- a/gcc/common/config/riscv/riscv-common.cc >+++ b/gcc/common/config/riscv/riscv-common.cc >@@ -375,7 +375,7 @@ riscv_subset_list::add (const char *subset, int > major_version, > else > error_at ( > m_loc, >-"%<-march=%s%>: extension %qs appear more than one time", >+"%<-march=%s%>: extension %qs appears more than one time", Or "... more than once" even. Maciej
[PATCH] PR fortran/104128 - ICE in gfc_widechar_to_char, at fortran/scanner.c:199
Dear Fortranners, conversions between different character kinds using TRANSFER exhibit inconsistencies that can occur between expr->representation.string (which is char*) on the one hand, and expr->->value.character.string. One issue (in target-memory.cc) is easily fixed by simply passing a conversion flag that was likely forgotten in the past. The other issue happens in gfc_copy_expr. Before we unconditionally converted an existing representation.string to wide char, which is definitely wrong. Restricting that code path to default character kind fixed the problems I could find and produces dumps that looked fine to me. Maybe some expert here can find a better fix. Regtested on x86_64-pc-linux-gnu. OK for mainline? Maybe 11-branch? Thanks, Harald From ddf161bd2b4de1c0a9655cb61634d94c857b458b Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Sun, 23 Jan 2022 21:55:33 +0100 Subject: [PATCH] Fortran: fix issues with internal conversion between default and wide char gcc/fortran/ChangeLog: PR fortran/104128 * expr.cc (gfc_copy_expr): Convert internal representation of string to wide char in value only for default character kind. * target-memory.cc (interpret_array): Pass flag for conversion of wide chars. (gfc_target_interpret_expr): Likewise. gcc/testsuite/ChangeLog: PR fortran/104128 * gfortran.dg/transfer_simplify_14.f90: New test. --- gcc/fortran/expr.cc | 3 ++- gcc/fortran/target-memory.cc | 7 ++--- .../gfortran.dg/transfer_simplify_14.f90 | 27 +++ 3 files changed, 33 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/transfer_simplify_14.f90 diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc index 279d9b30991..ed82a94022f 100644 --- a/gcc/fortran/expr.cc +++ b/gcc/fortran/expr.cc @@ -312,7 +312,8 @@ gfc_copy_expr (gfc_expr *p) break; case BT_CHARACTER: - if (p->representation.string) + if (p->representation.string + && p->ts.kind == gfc_default_character_kind) q->value.character.string = gfc_char_to_widechar (q->representation.string); else diff --git a/gcc/fortran/target-memory.cc b/gcc/fortran/target-memory.cc index 361907b0e51..7ce7d736629 100644 --- a/gcc/fortran/target-memory.cc +++ b/gcc/fortran/target-memory.cc @@ -365,7 +365,8 @@ gfc_target_encode_expr (gfc_expr *source, unsigned char *buffer, static size_t -interpret_array (unsigned char *buffer, size_t buffer_size, gfc_expr *result) +interpret_array (unsigned char *buffer, size_t buffer_size, gfc_expr *result, + bool convert_widechar) { gfc_constructor_base base = NULL; size_t array_size = 1; @@ -390,7 +391,7 @@ interpret_array (unsigned char *buffer, size_t buffer_size, gfc_expr *result) gfc_constructor_append_expr (&base, e, &result->where); ptr += gfc_target_interpret_expr (&buffer[ptr], buffer_size - ptr, e, - true); + convert_widechar); } result->value.constructor = base; @@ -580,7 +581,7 @@ gfc_target_interpret_expr (unsigned char *buffer, size_t buffer_size, gfc_expr *result, bool convert_widechar) { if (result->expr_type == EXPR_ARRAY) -return interpret_array (buffer, buffer_size, result); +return interpret_array (buffer, buffer_size, result, convert_widechar); switch (result->ts.type) { diff --git a/gcc/testsuite/gfortran.dg/transfer_simplify_14.f90 b/gcc/testsuite/gfortran.dg/transfer_simplify_14.f90 new file mode 100644 index 000..dfb997d81b2 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/transfer_simplify_14.f90 @@ -0,0 +1,27 @@ +! { dg-do compile } +! { dg-options "-fdump-tree-original" } +! PR fortran/104128 - ICE in gfc_widechar_to_char +! Contributed by G.Steinmetz + +program p + implicit none + integer, parameter :: k = 4 + character(*), parameter :: a = 'abc' + character(*,kind=4), parameter :: b = 'abc' + character(2,kind=k), parameter :: s = k_"FG" + character(*,kind=1), parameter :: x = transfer (s, 'abcdefgh') + character(2,kind=k), parameter :: t = transfer (x, s) + character(2,kind=k):: u = transfer (x, s) + logical, parameter :: l = (s == t) + print *, transfer (a , 4_'xy', size=2) + print *, transfer ('xyz', [b], size=2) + print *, s + print *, t + print *, u + if (.not. l) stop 1 + if (t /= s) stop 2 + if (u /= s) stop 3 ! not optimized away +end + +! { dg-final { scan-tree-dump-times "_gfortran_stop_numeric" 1 "original" } } +! { dg-final { scan-tree-dump "_gfortran_stop_numeric \\(3, 0\\);" "original" } } -- 2.31.1
Re: [PATCH] libgccjit: Add option to hide stderr logs [PR104073]
Thanks for the review. Here's the updated patch. Le mardi 18 janvier 2022 à 18:22 -0500, David Malcolm a écrit : > On Mon, 2022-01-17 at 21:02 -0500, Antoni Boucher via Gcc-patches > wrote: > > Hi. > > This option will be useful for rustc_codegen_gcc to hide the error > > about unsupported 128-bit integer types. > > > > David, if you know of a better way to check if these types are > > supported than creating such a type and checking if it causes an > > error, > > I will not need this patch. > > Off the top of my head I don't know of such a way. > > That said, this seems to be vaguely analogous to a test in a > "configure" script, attempting a compile and seeing if it succeeds. > > This seems like a useful pattern for libgccjit to support, so that > client code can query the capabilities of the host, so I think the > idea > of this patch is sound. > > As for the details of the patch, I don't like adding new members to > the > enums in libgccjit.h; I prefer adding new entrypoints, as the latter > gives a way to tell if client code uses the new entrypoint as part of > the ELF metadata, so that we can tell directly that client code is > incompatible with an older libgccjit.so from the symbol metadata in > the > built binary. > > So I'd prefer something like: > > extern void > gcc_jit_context_set_bool_print_errors_to_stderr (gcc_jit_context > *ctxt, > int enabled); > > where gcc_jit_context_set_bool_print_errors_to_stderr defaults to > true, > but client code can use: > > gcc_jit_context_set_bool_print_errors_to_stderr (ctxt, false); > > Or maybe have a way to specify the FILE * for errors to be printed > to, > defaulting to stderr, but settable to NULL if you want to suppress > the > printing? That might be more flexible. > > Thoughts? > Dave > From 1f1b7d2298956268e2678a157a34c8f9ac370f3a Mon Sep 17 00:00:00 2001 From: Antoni Boucher Date: Sun, 23 Jan 2022 11:37:07 -0500 Subject: [PATCH] libgccjit: Add function to hide stderr logs [PR104073] 2022-01-23 Antoni Boucher get_debug_string (), - errmsg); - else -fprintf (stderr, "%s: error: %s\n", - ctxt_progname, - errmsg); + bool print_errors_to_stderr = + get_inner_bool_option (INNER_BOOL_OPTION_PRINT_ERRORS_TO_STDERR); + if (print_errors_to_stderr) + { +if (loc) + fprintf (stderr, "%s: %s: error: %s\n", + ctxt_progname, + loc->get_debug_string (), + errmsg); +else + fprintf (stderr, "%s: error: %s\n", + ctxt_progname, + errmsg); + } if (!m_error_count) { @@ -1687,7 +1693,8 @@ static const char * const static const char * const inner_bool_option_reproducer_strings[NUM_INNER_BOOL_OPTIONS] = { "gcc_jit_context_set_bool_allow_unreachable_blocks", - "gcc_jit_context_set_bool_use_external_driver" + "gcc_jit_context_set_bool_use_external_driver", + "gcc_jit_context_set_bool_print_errors_to_stderr", }; /* Write the current value of all options to the log file (if any). */ diff --git a/gcc/jit/libgccjit.cc b/gcc/jit/libgccjit.cc index 4c352e8c93d..778f8e926a2 100644 --- a/gcc/jit/libgccjit.cc +++ b/gcc/jit/libgccjit.cc @@ -3431,6 +3431,23 @@ gcc_jit_context_set_bool_allow_unreachable_blocks (gcc_jit_context *ctxt, bool_value); } +/* Public entrypoint. See description in libgccjit.h. + + After error-checking, the real work is done by the + gcc::jit::recording::context::set_inner_bool_option method in + jit-recording.cc. */ + +void +gcc_jit_context_set_bool_print_errors_to_stderr (gcc_jit_context *ctxt, + int enabled) +{ + RETURN_IF_FAIL (ctxt, NULL, NULL, "NULL context"); + JIT_LOG_FUNC (ctxt->get_logger ()); + ctxt->set_inner_bool_option ( +gcc::jit::INNER_BOOL_OPTION_PRINT_ERRORS_TO_STDERR, +enabled); +} + /* Public entrypoint. See description in libgccjit.h. After error-checking, the real work is done by the diff --git a/gcc/jit/libgccjit.h b/gcc/jit/libgccjit.h index 2a5ffacb1fe..16b8637dd0a 100644 --- a/gcc/jit/libgccjit.h +++ b/gcc/jit/libgccjit.h @@ -293,6 +293,24 @@ gcc_jit_context_set_bool_allow_unreachable_blocks (gcc_jit_context *ctxt, tested for with #ifdef. */ #define LIBGCCJIT_HAVE_gcc_jit_context_set_bool_allow_unreachable_blocks +/* By default, libgccjit will print errors to stderr. + + This option can be used to disable the printing. + + This entrypoint was added in LIBGCCJIT_ABI_23; you can test for + its presence using + #ifdef LIBGCCJIT_HAVE_gcc_jit_context_set_bool_print_errors_to_stderr +*/ + +extern void +gcc_jit_context_set_bool_print_errors_to_stderr (gcc_jit_context *ctxt, + int enabled); + +/* Pre-canned feature macro to indicate the presence of + gcc_jit_context_set_bool_allow_unreachable_blocks. This can be + tested for with #ifdef. */ +#define LIBGCCJIT_HAVE_gcc_jit_context_set_bool_print_errors_to_stderr + /* Implementation detail: libgccjit internally generates assembler, and uses "driv
Re: [committed] analyzer: reject ((i + 1 > 0) && (i < 0)) for integers [PR94362]
Hello, Le 21/01/2022 à 00:59, David Malcolm via Gcc-patches a écrit : diff --git a/gcc/analyzer/constraint-manager.cc b/gcc/analyzer/constraint-manager.cc index 568e7150ea7..7c4a85bbb24 100644 --- a/gcc/analyzer/constraint-manager.cc +++ b/gcc/analyzer/constraint-manager.cc @@ -301,6 +301,80 @@ range::above_upper_bound (tree rhs_const) const m_upper_bound.m_constant).is_true (); } +/* Attempt to add B to the bound of the given kind of this range. + Return true if feasible; false if infeasible. */ + +bool +range::add_bound (bound b, enum bound_kind bound_kind) +{ + b.ensure_closed (bound_kind); + + switch (bound_kind) +{ +default: + gcc_unreachable (); +case BK_LOWER: + /* Discard redundant bounds. */ + if (m_lower_bound.m_constant) + { + m_lower_bound.ensure_closed (BK_LOWER); + if (!tree_int_cst_lt (b.m_constant, + m_lower_bound.m_constant)) + return true; isn’t this condition reversed? + } + m_lower_bound = b; + break; +case BK_UPPER: + /* Discard redundant bounds. */ + if (m_upper_bound.m_constant) + { + m_upper_bound.ensure_closed (BK_UPPER); + if (tree_int_cst_le (b.m_constant, + m_upper_bound.m_constant)) + return true; same here. All the tests added have just one lower and one upper bound, so they don’t use the short-circuit code, but amending one of them as follows makes the problem appear as the test starts to fails. It should continue to work, shouldn’t it? diff --git a/gcc/analyzer/constraint-manager.cc b/gcc/analyzer/constraint-manager.cc index 7c4a85bbb24..3f38b857722 100644 --- a/gcc/analyzer/constraint-manager.cc +++ b/gcc/analyzer/constraint-manager.cc @@ -3697,6 +3697,7 @@ test_constant_comparisons () region_model_manager mgr; { region_model model (&mgr); + ADD_SAT_CONSTRAINT (model, int_1, LT_EXPR, a); ADD_SAT_CONSTRAINT (model, int_3, LT_EXPR, a); ADD_UNSAT_CONSTRAINT (model, a, LT_EXPR, int_4); }
Re: [PATCH] Fortran: detect signaling NaNs on targets without issignaling macro in libc
Le 23/01/2022 à 11:05, FX a écrit : Hi Mikael, I spotted two unexpected things (to me at least) related to x86 extended type: - You check for endianness, so the format is not x86-specific? - Is it expected that the padding bits are in the middle of the data in the big-endian case? IEEE specifies that extended precision types can be present, possibly with any endianness, at least in theory. There are other CPUs with extended precision types than Intel, although we probably don’t support them currently in gfortran: Motorola 68881 has a IEEE-compatible 80-bit format and is big endian. I kept the code generic, but if you think it’s a problem I can remove that part and make it error out. No, it’s not a problem, I just was surprised to see endianness checks as I thought it was x86-only. I followed the logic used in glibc to deal with bit layout and endianness, so it should be safe as currently proposed. Then it’s OK to commit for me, but you will need approval from release managers at this stage. Thanks.
Re: [PATCH] gcc: pass-manager: Fix memory leak. [PR jit/63854]
Am Sa., 15. Jan. 2022 um 14:56 Uhr schrieb Marc Nieper-Wißkirchen : > > Jeff, David, do you need any more input from my side? > > -- Marc > > Am Sa., 8. Jan. 2022 um 17:32 Uhr schrieb Jeff Law : > > > > > > > > On 1/6/2022 6:53 AM, David Malcolm via Gcc-patches wrote: > > > On Sun, 2021-12-19 at 22:30 +0100, Marc Nieper-Wißkirchen wrote: > > >> This patch fixes a memory leak in the pass manager. In the existing > > >> code, > > >> the m_name_to_pass_map is allocated in > > >> pass_manager::register_pass_name, but > > >> never deallocated. This is fixed by adding a deletion in > > >> pass_manager::~pass_manager. Moreover the string keys in > > >> m_name_to_pass_map are > > >> all dynamically allocated. To free them, this patch adds a new hash > > >> trait for > > >> string hashes that are to be freed when the corresponding hash entry > > >> is removed. > > >> > > >> This fix is particularly relevant for using GCC as a library through > > >> libgccjit. > > >> The memory leak also occurs when libgccjit is instructed to use an > > >> external > > >> driver. > > >> > > >> Before the patch, compiling the hello world example of libgccjit with > > >> the external driver under Valgrind shows a loss of 12,611 (48 direct) > > >> bytes. After the patch, no memory leaks are reported anymore. > > >> (Memory leaks occurring when using the internal driver are mostly in > > >> the driver code in gcc/gcc.c and have to be fixed separately.) > > >> > > >> The patch has been tested by fully bootstrapping the compiler with > > >> the > > >> frontends C, C++, Fortran, LTO, ObjC, JIT and running the test suite > > >> under a x86_64-pc-linux-gnu host. > > > Thanks for the patch. > > > > > > It looks correct to me, given that pass_manager::register_pass_name > > > does an xstrdup and puts the result in the map. > > > > > > That said: > > > - I'm not officially a reviewer for this part of gcc (though I probably > > > touched this code last) > > > - is it cleaner to instead change m_name_to_pass_map's key type from > > > const char * to char *, to convey that the map "owns" the name? That > > > way we probably wouldn't need struct typed_const_free_remove, and (I > > > hope) works better with the type system. > > > > > > Dave > > > > > >> gcc/ChangeLog: > > >> > > >> PR jit/63854 > > >> * hash-traits.h (struct typed_const_free_remove): New. > > >> (struct free_string_hash): New. > > >> * pass_manager.h: Use free_string_hash. > > >> * passes.c (pass_manager::register_pass_name): Use > > >> free_string_hash. > > >> (pass_manager::~pass_manager): Delete allocated > > >> m_name_to_pass_map. > > My concern (and what I hadn't had time to dig into) was we initially > > used nofree_string_hash -- I wanted to make sure there wasn't any path > > where the name came from the stack (can't be free'd), was saved > > elsewhere (danging pointer) and the like. ie, why were we using > > nofree_string_hash to begin with? I've never really mucked around with > > these bits, so the analysis side kept falling off the daily todo list. The only occurrences of m_name_to_pass_map are in pass-manager.h (where it is defined as a private field of the class pass_manager) and in passes.cc. There is just one instance where a name is added to the map in passes.cc, namely through the put method. There, the name has been xstrdup'ed. The name (as a const char *) escapes the pass map in pass_manager::create_pass_tab through the call to m_name_pass_map->traverse. This inserts the name into the pass_tab, which is a static vec of const char *s. The pass_tab does not escape the translation unit of passes.c. It is used in dump_one_pass where the name is used as an argument to fprintf. The important point is that it is not freed and not further copied. > > > > If/once you're comfortable with the patch David, then go ahead and apply > > it on Marc's behalf. > > > > jeff > >
[PATCH v2] x86: Also check mode of memory broadcast in bcst_mem_operand
Return false for invalid mode on memory broadcast in bcst_mem_operand: (vec_duplicate:V16SF (mem/j:V4SF (reg/v/f:DI 109 [ b ]))) gcc/ PR target/104188 * config/i386/predicates.md (bcst_mem_operand): Also check mode of memory broadcast. gcc/testsuite/ PR target/104188 * gcc.target/i386/pr104188.c: New test. --- gcc/config/i386/predicates.md| 2 + gcc/testsuite/gcc.target/i386/pr104188.c | 70 2 files changed, 72 insertions(+) create mode 100644 gcc/testsuite/gcc.target/i386/pr104188.c diff --git a/gcc/config/i386/predicates.md b/gcc/config/i386/predicates.md index eae6ab58e23..a8cc17a054d 100644 --- a/gcc/config/i386/predicates.md +++ b/gcc/config/i386/predicates.md @@ -1157,6 +1157,8 @@ (define_predicate "bcst_mem_operand" (ior (match_test "TARGET_AVX512VL") (match_test "GET_MODE_SIZE (GET_MODE (op)) == 64"))) (match_test "VALID_BCST_MODE_P (GET_MODE_INNER (GET_MODE (op)))") + (match_test "GET_MODE (XEXP (op, 0)) + == GET_MODE_INNER (GET_MODE (op))") (match_test "memory_operand (XEXP (op, 0), GET_MODE (XEXP (op, 0)))"))) ; Return true when OP is bcst_mem_operand or vector_memory_operand. diff --git a/gcc/testsuite/gcc.target/i386/pr104188.c b/gcc/testsuite/gcc.target/i386/pr104188.c new file mode 100644 index 000..c6f615b9625 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr104188.c @@ -0,0 +1,70 @@ +/* { dg-do run { target avx512f } } */ +/* { dg-options "-O2 -mfpmath=sse" } */ + +#include + +union U { + float m[4][4]; + __m128 r[4]; + __m512 s; +}; + +__attribute__((noipa, target("avx512f"))) +void +foo (union U *x, union U *a, union U *b) +{ + __m512 c = _mm512_loadu_ps (&a->s); + __m512 d = _mm512_broadcast_f32x4 (b->r[0]); + __m512 e = _mm512_broadcast_f32x4 (b->r[1]); + __m512 f = _mm512_broadcast_f32x4 (b->r[2]); + __m512 g = _mm512_broadcast_f32x4 (b->r[3]); + __m512 h = _mm512_mul_ps (_mm512_permute_ps (c, 0x00), d); + h = _mm512_fmadd_ps (_mm512_permute_ps (c, 0x55), e, h); + h = _mm512_fmadd_ps (_mm512_permute_ps (c, 0xaa), f, h); + h = _mm512_fmadd_ps (_mm512_permute_ps (c, 0xff), g, h); + _mm512_storeu_ps (&x->s, h); +} + +__attribute__((noipa, target("avx512f"))) +void +do_test (void) +{ + union U a = { .m = { { 1.0f, 2.0f, 3.0f, 4.0f }, + { 5.0f, 6.0f, 7.0f, 8.0f }, + { 9.0f, 10.0f, 11.0f, 12.0f }, + { 13.0f, 14.0f, 15.0f, 16.0f } } }; + union U b = { .m = { { 17.0f, 18.0f, 19.0f, 20.0f }, + { 21.0f, 22.0f, 23.0f, 24.0f }, + { 25.0f, 26.0f, 27.0f, 28.0f }, + { 29.0f, 30.0f, 31.0f, 32.0f } } }; + union U c; + foo (&c, &a, &b); + if (c.m[0][0] != 250.0f + || c.m[0][1] != 260.0f + || c.m[0][2] != 270.0f + || c.m[0][3] != 280.0f) +__builtin_abort (); + if (c.m[1][0] != 618.0f + || c.m[1][1] != 644.0f + || c.m[1][2] != 670.0f + || c.m[1][3] != 696.0f) +__builtin_abort (); + if (c.m[2][0] != 986.0f + || c.m[2][1] != 1028.0f + || c.m[2][2] != 1070.0f + || c.m[2][3] != 1112.0f) +__builtin_abort (); + if (c.m[3][0] != 1354.0f + || c.m[3][1] != 1412.0f + || c.m[3][2] != 1470.0f + || c.m[3][3] != 1528.0f) +__builtin_abort (); +} + +int +main () +{ + if (__builtin_cpu_supports ("avx512f")) +do_test (); + return 0; +} -- 2.34.1
Re: [PATCH] Disable -fsplit-stack support on non-glibc targets
On Sun, Jan 23, 2022 at 10:06:24AM +0100, Uros Bizjak wrote: > > 2022-01-22 Jakub Jelinek > > > > * config/linux.h (OPTION_GLIBC_P, OPTION_UCLIBC_P, > > OPTION_BIONIC_P, OPTION_MUSL_P): Define. > > (OPTION_GLIBC, OPTION_UCLIBC, OPTION_BIONIC, OPTION_MUSL): Redefine > > using OPTION_*_P macros. > > * config/alpha/linux.h (OPTION_GLIBC_P, OPTION_UCLIBC_P, > > OPTION_BIONIC_P, OPTION_MUSL_P): Define. > > (OPTION_GLIBC, OPTION_UCLIBC, OPTION_BIONIC, OPTION_MUSL): Redefine > > using OPTION_*_P macros. > > * config/rs6000/linux.h (OPTION_GLIBC_P, OPTION_UCLIBC_P, > > OPTION_BIONIC_P, OPTION_MUSL_P): Define. > > (OPTION_GLIBC, OPTION_UCLIBC, OPTION_BIONIC, OPTION_MUSL): Redefine > > using OPTION_*_P macros. > > * config/rs6000/linux64.h (OPTION_GLIBC_P, OPTION_UCLIBC_P, > > OPTION_BIONIC_P, OPTION_MUSL_P): Define. > > (OPTION_GLIBC, OPTION_UCLIBC, OPTION_BIONIC, OPTION_MUSL): Redefine > > using OPTION_*_P macros. > > * config/fuchsia.h (OPTION_MUSL_P): Redefine. > > * config/glibc-stdint.h (OPTION_MUSL_P): Define if not defined. > > * common/config/s390/s390-common.cc (s390_supports_split_stack): > > Re-add > > ATTRIBUTE_UNUSED to opts parameter. If OPTION_GLIBC_P is defined, > > use > > OPTION_GLIBC_P (opts) as condition, otherwise assume if (false). > > * common/config/i386/i386-common.cc (ix86_supports_split_stack): If > > OPTION_GLIBC_P is defined use !OPTION_GLIBC_P (opts) as condition, > > otherwise assume if (true). > > I wonder why every target defines its own set of #defines. I'd expect > that they include toplevel gcc/config/linux.h and inherit these > defines from it. Not every target, most of the Linux targets do use config/linux.h, just ppc* and alpha haven't been converted to do so. It would be nice to have that done at some point, but to me that looks like stage1 task rather than stage4. Jakub
Re: [PATCH] Fortran: detect signaling NaNs on targets without issignaling macro in libc
Hi Mikael, > I spotted two unexpected things (to me at least) related to x86 extended type: > - You check for endianness, so the format is not x86-specific? > - Is it expected that the padding bits are in the middle of the data in the > big-endian case? IEEE specifies that extended precision types can be present, possibly with any endianness, at least in theory. There are other CPUs with extended precision types than Intel, although we probably don’t support them currently in gfortran: Motorola 68881 has a IEEE-compatible 80-bit format and is big endian. I kept the code generic, but if you think it’s a problem I can remove that part and make it error out. I followed the logic used in glibc to deal with bit layout and endianness, so it should be safe as currently proposed. FX
Re: [PATCH v5 02/12] LoongArch Port: gcc build
在 2022/1/23 下午5:00, Xi Ruoyao 写道: On Sun, 2022-01-23 at 16:39 +0800, 程璐璐 wrote: 在 2022/1/22 下午4:42, Xi Ruoyao 写道: On Sat, 2022-01-22 at 15:55 +0800, Chenghua Xu wrote: +mstrict-align +Target Var(TARGET_STRICT_ALIGN) Init(0) +Do not generate unaligned memory accesses. Section 2.1.8 of LoongArch spec says "load/store instruction *may* be implemented to allow unaligned memory access". As it's not a "must", should we really enable this by default? Currently, our main architecture supports this feature. Later, in the compiler the value of this macro will be set according to the architecture. My suggestion is to make -mstrict-align default for the default ('-march=loongarch64'), as it should generate code working on every architecture following the spec. And, -mno-strict-align should be the default of -march=la464, which is the current main architecture supporting unaligned access. Ok we will consider your suggestion!
Re: Difference between 32-bit SPARCv9 and SPARCv8+?
> Does anyone know the reasoning behind this? Solaris preserves the full 64-bit registers in 32-bit mode. -- Eric Botcazou
Re: Difference between 32-bit SPARCv9 and SPARCv8+?
> While playing around with the compiler options trying to find a solution, I > made an interesting discovery which is that GCC support 64-bit compare and > swap on SPARCv8plus but not on 32-bit SPARCv9: > > glaubitz@gcc202:~$ echo | gcc -mv8plus -E -dM -|grep -i SWAP > #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_1 1 > #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_2 1 > #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4 1 > #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8 1 > glaubitz@gcc202:~$ echo | gcc -mcpu=v9 -m32 -E -dM -|grep -i SWAP > #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_1 1 > #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_2 1 > #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4 1 > glaubitz@gcc202:~$ > > Is this intentional? If yes, what is the exact difference between V8+ and > 32-bit V9? V8+ requires the full 64-bit registers to be preserved by the kernel, whereas 32-bit V9 does not. -- Eric Botcazou
Re: [PATCH] Disable -fsplit-stack support on non-glibc targets
On Sat, Jan 22, 2022 at 7:04 PM Jakub Jelinek via Gcc-patches wrote: > > On Sat, Jan 22, 2022 at 01:16:38PM +0100, Jakub Jelinek via Gcc-patches wrote: > > Actually, I suspect we either need something like following patch, > > or need to change gcc/config/{linux,rs6000/linux{,64},alpha/linux}.h > > so that next to those OPTION_GLIBC etc. macros it also defines versions > > of those macros with opts argument. > > And here is a larger but perhaps cleaner patch that matches how e.g. > options.h defines TARGET_WHATEVER_P(opts) options and then TARGET_WHATEVER > too. > > Only compile tested on x86_64-linux so far. > > 2022-01-22 Jakub Jelinek > > * config/linux.h (OPTION_GLIBC_P, OPTION_UCLIBC_P, > OPTION_BIONIC_P, OPTION_MUSL_P): Define. > (OPTION_GLIBC, OPTION_UCLIBC, OPTION_BIONIC, OPTION_MUSL): Redefine > using OPTION_*_P macros. > * config/alpha/linux.h (OPTION_GLIBC_P, OPTION_UCLIBC_P, > OPTION_BIONIC_P, OPTION_MUSL_P): Define. > (OPTION_GLIBC, OPTION_UCLIBC, OPTION_BIONIC, OPTION_MUSL): Redefine > using OPTION_*_P macros. > * config/rs6000/linux.h (OPTION_GLIBC_P, OPTION_UCLIBC_P, > OPTION_BIONIC_P, OPTION_MUSL_P): Define. > (OPTION_GLIBC, OPTION_UCLIBC, OPTION_BIONIC, OPTION_MUSL): Redefine > using OPTION_*_P macros. > * config/rs6000/linux64.h (OPTION_GLIBC_P, OPTION_UCLIBC_P, > OPTION_BIONIC_P, OPTION_MUSL_P): Define. > (OPTION_GLIBC, OPTION_UCLIBC, OPTION_BIONIC, OPTION_MUSL): Redefine > using OPTION_*_P macros. > * config/fuchsia.h (OPTION_MUSL_P): Redefine. > * config/glibc-stdint.h (OPTION_MUSL_P): Define if not defined. > * common/config/s390/s390-common.cc (s390_supports_split_stack): > Re-add > ATTRIBUTE_UNUSED to opts parameter. If OPTION_GLIBC_P is defined, use > OPTION_GLIBC_P (opts) as condition, otherwise assume if (false). > * common/config/i386/i386-common.cc (ix86_supports_split_stack): If > OPTION_GLIBC_P is defined use !OPTION_GLIBC_P (opts) as condition, > otherwise assume if (true). I wonder why every target defines its own set of #defines. I'd expect that they include toplevel gcc/config/linux.h and inherit these defines from it. Uros. > > --- gcc/config/linux.h.jj 2022-01-18 11:58:59.160988086 +0100 > +++ gcc/config/linux.h 2022-01-22 18:42:25.476235564 +0100 > @@ -29,18 +29,23 @@ see the files COPYING3 and COPYING.RUNTI > > /* C libraries supported on Linux. */ > #ifdef SINGLE_LIBC > -#define OPTION_GLIBC (DEFAULT_LIBC == LIBC_GLIBC) > -#define OPTION_UCLIBC (DEFAULT_LIBC == LIBC_UCLIBC) > -#define OPTION_BIONIC (DEFAULT_LIBC == LIBC_BIONIC) > -#undef OPTION_MUSL > -#define OPTION_MUSL (DEFAULT_LIBC == LIBC_MUSL) > +#define OPTION_GLIBC_P(opts) (DEFAULT_LIBC == LIBC_GLIBC) > +#define OPTION_UCLIBC_P(opts) (DEFAULT_LIBC == LIBC_UCLIBC) > +#define OPTION_BIONIC_P(opts) (DEFAULT_LIBC == LIBC_BIONIC) > +#undef OPTION_MUSL_P > +#define OPTION_MUSL_P(opts)(DEFAULT_LIBC == LIBC_MUSL) > #else > -#define OPTION_GLIBC (linux_libc == LIBC_GLIBC) > -#define OPTION_UCLIBC (linux_libc == LIBC_UCLIBC) > -#define OPTION_BIONIC (linux_libc == LIBC_BIONIC) > -#undef OPTION_MUSL > -#define OPTION_MUSL (linux_libc == LIBC_MUSL) > +#define OPTION_GLIBC_P(opts) ((opts)->x_linux_libc == LIBC_GLIBC) > +#define OPTION_UCLIBC_P(opts) ((opts)->x_linux_libc == LIBC_UCLIBC) > +#define OPTION_BIONIC_P(opts) ((opts)->x_linux_libc == LIBC_BIONIC) > +#undef OPTION_MUSL_P > +#define OPTION_MUSL_P(opts)((opts)->x_linux_libc == LIBC_MUSL) > #endif > +#define OPTION_GLIBC OPTION_GLIBC_P (&global_options) > +#define OPTION_UCLIBC OPTION_UCLIBC_P (&global_options) > +#define OPTION_BIONIC OPTION_BIONIC_P (&global_options) > +#undef OPTION_MUSL > +#define OPTION_MUSLOPTION_MUSL_P (&global_options) > > #define GNU_USER_TARGET_OS_CPP_BUILTINS() \ > do { \ > --- gcc/config/alpha/linux.h.jj 2022-01-11 23:11:21.692299963 +0100 > +++ gcc/config/alpha/linux.h2022-01-22 18:43:59.739923743 +0100 > @@ -58,18 +58,23 @@ along with GCC; see the file COPYING3. > #define WCHAR_TYPE "int" > > #ifdef SINGLE_LIBC > -#define OPTION_GLIBC (DEFAULT_LIBC == LIBC_GLIBC) > -#define OPTION_UCLIBC (DEFAULT_LIBC == LIBC_UCLIBC) > -#define OPTION_BIONIC (DEFAULT_LIBC == LIBC_BIONIC) > -#undef OPTION_MUSL > -#define OPTION_MUSL (DEFAULT_LIBC == LIBC_MUSL) > +#define OPTION_GLIBC_P(opts) (DEFAULT_LIBC == LIBC_GLIBC) > +#define OPTION_UCLIBC_P(opts) (DEFAULT_LIBC == LIBC_UCLIBC) > +#define OPTION_BIONIC_P(opts) (DEFAULT_LIBC == LIBC_BIONIC) > +#undef OPTION_MUSL_P > +#define OPTION_MUSL_P(opts)(DEFAULT_LIBC == LIBC_MUSL) > #else > -#define OPTION_GLIBC (linux_libc == LIBC_GLIBC) > -#define OPTION_UCLIBC (linux_libc == LIBC_UCLIBC) > -#define OPTION_BIONIC (linux_libc == LIBC_BIONIC
Re: [PATCH v5 02/12] LoongArch Port: gcc build
On Sun, 2022-01-23 at 16:39 +0800, 程璐璐 wrote: > > 在 2022/1/22 下午4:42, Xi Ruoyao 写道: > > > > On Sat, 2022-01-22 at 15:55 +0800, Chenghua Xu wrote: > > > > > > > +mstrict-align > > > +Target Var(TARGET_STRICT_ALIGN) Init(0) > > > +Do not generate unaligned memory accesses. > > Section 2.1.8 of LoongArch spec says "load/store instruction *may* be > > implemented to allow unaligned memory access". As it's not a "must", > > should we really enable this by default? > Currently, our main architecture supports this feature. Later, in the compiler > the value of this macro will be set according to the architecture. My suggestion is to make -mstrict-align default for the default ('-march=loongarch64'), as it should generate code working on every architecture following the spec. And, -mno-strict-align should be the default of -march=la464, which is the current main architecture supporting unaligned access. -- Xi Ruoyao School of Aerospace Science and Technology, Xidian University
Re: [PATCH v5 02/12] LoongArch Port: gcc build
在 2022/1/22 下午4:42, Xi Ruoyao 写道: On Sat, 2022-01-22 at 15:55 +0800, Chenghua Xu wrote: +mstrict-align +Target Var(TARGET_STRICT_ALIGN) Init(0) +Do not generate unaligned memory accesses. Section 2.1.8 of LoongArch spec says "load/store instruction *may* be implemented to allow unaligned memory access". As it's not a "must", should we really enable this by default? Currently, our main architecture supports this feature. Later, in the compiler the value of this macro will be set according to the architecture.
Re: [PATCH v5 02/12] LoongArch Port: gcc build
在 2022/1/22 下午5:31, Jakub Jelinek 写道: On Sat, Jan 22, 2022 at 05:05:00PM +0800, Xi Ruoyao wrote: On Sat, 2022-01-22 at 16:56 +0800, 程璐璐 wrote: Under the MIPS architecture, *.opt files are also generated in $(srcdir). Well, but then you should put the commands for generating those files into contrib/gcc_update instead of Makefile.in, just like MIPS. The MIPS does it wrong too. If the generated file is checked into the tree as in the mips case, such a make rule can be there, but it should be wrapped with ifeq($(ENABLE_MAINTAINER_RULES),true) ... endif or so. The point is, when not --enable-maintainer-mode, the gcc tree can be stored on read-only filesystem. When adding new CPUs, one just uses --enable-maintainer-mode and regenates the generated but committed files and commits them together with the changes. There are many other examples of such generated files, configure, Makefile.in from Makefile.am, etc. Jakub Hi, Mr Jakub: I have fixed it, could you please take a look. diff --git a/contrib/gcc_update b/contrib/gcc_update index 1cf15f9b3c2..641ce164775 100755 --- a/contrib/gcc_update +++ b/contrib/gcc_update @@ -86,6 +86,8 @@ gcc/config/arm/arm-tables.opt: gcc/config/arm/arm-cpus.in gcc/config/arm/parsecp gcc/config/c6x/c6x-tables.opt: gcc/config/c6x/c6x-isas.def gcc/config/c6x/genopt.sh gcc/config/c6x/c6x-sched.md: gcc/config/c6x/c6x-sched.md.in gcc/config/c6x/gensched.sh gcc/config/c6x/c6x-mult.md: gcc/config/c6x/c6x-mult.md.in gcc/config/c6x/genmult.sh +gcc/config/loongarch/loongarch-str.h: gcc/config/loongarch/genopts/genstr.sh gcc/config/loongarch/genopts/loongarch-string +gcc/config/loongarch/loongarch.opt: gcc/config/loongarch/genopts/genstr.sh gcc/config/loongarch/genopts/loongarch.opt.in gcc/config/m68k/m68k-tables.opt: gcc/config/m68k/m68k-devices.def gcc/config/m68k/m68k-isas.def gcc/config/m68k/m68k-microarchs.def gcc/config/m68k/genopt.sh gcc/config/mips/mips-tables.opt: gcc/config/mips/mips-cpus.def gcc/config/mips/genopt.sh gcc/config/rs6000/rs6000-tables.opt: gcc/config/rs6000/rs6000-cpus.def gcc/config/rs6000/genopt.sh diff --git a/gcc/config/loongarch/t-loongarch b/gcc/config/loongarch/t-loongarch index 6ed1a3ab56a..c106be1ec45 100644 --- a/gcc/config/loongarch/t-loongarch +++ b/gcc/config/loongarch/t-loongarch @@ -21,7 +21,15 @@ LA_MULTIARCH_TRIPLET = $(patsubst LA_MULTIARCH_TRIPLET=%,%,$\ $(filter LA_MULTIARCH_TRIPLET=%,$(tm_defines))) # String definition header -LA_STR_H = $(srcdir)/config/loongarch/loongarch-str.h +$(srcdir)/config/loongarch/loongarch-str.h: s-loongarch-str ; @true +s-loongarch-str: $(srcdir)/config/loongarch/genopts/genstr.sh \ + $(srcdir)/config/loongarch/genopts/loongarch-strings + $(SHELL) $(srcdir)/config/loongarch/genopts/genstr.sh header \ + $(srcdir)/config/loongarch/genopts/loongarch-strings > \ + tmp-loongarch-str.h + $(SHELL) $(srcdir)/../move-if-change tmp-loongarch-str.h \ + $(srcdir)/config/loongarch/loongarch-str.h + $(STAMP) s-loongarch-str loongarch-c.o: $(srcdir)/config/loongarch/loongarch-c.cc $(CONFIG_H) $(SYSTEM_H) \ coretypes.h $(TM_H) $(TREE_H) output.h $(C_COMMON_H) $(TARGET_H) @@ -48,12 +56,13 @@ loongarch-cpu.o: $(srcdir)/config/loongarch/loongarch-cpu.cc $(LA_STR_H) loongarch-def.o: $(srcdir)/config/loongarch/loongarch-def.c $(LA_STR_H) $(CC) -c $(ALL_CFLAGS) $(INCLUDES) $< -$(srcdir)/config/loongarch/loongarch.opt: \ - $(srcdir)/config/loongarch/genopts/genstr.sh \ +$(srcdir)/config/loongarch/loongarch.opt: s-loongarch-opt ; @true +s-loongarch-opt: $(srcdir)/config/loongarch/genopts/genstr.sh \ $(srcdir)/config/loongarch/genopts/loongarch.opt.in - $(SHELL) $< opt > $@ + $(SHELL) $(srcdir)/config/loongarch/genopts/genstr.sh opt \ + $(srcdir)/config/loongarch/genopts/loongarch.opt.in \ + > tmp-loongarch.opt + $(SHELL) $(srcdir)/../move-if-change tmp-loongarch.opt \ + $(srcdir)/config/loongarch/loongarch.opt + $(STAMP) s-loongarch-opt -$(LA_STR_H): \ - $(srcdir)/config/loongarch/genopts/genstr.sh \ - $(srcdir)/config/loongarch/genopts/loongarch-strings - $(SHELL) $< header > $@