Re: [PATCH] RISC-V: Disable use of TLS copy relocs.
On Wed, 08 Jan 2020 17:05:21 PST (-0800), Jim Wilson wrote:
Musl and lld don't support TLS copy relocs, and don't want to add support
for this feature which is unique to RISC-V. Only GNU ld and glibc support
them. In the pasbi discussion, people have pointed out various problems
with using them, so we are deprecating them. There doesn't seem to be an
ABI break from dropping them so this patch modifies gcc to stop creating
them. I'm using an ifdef for now in case a problem turns up and the code
has to be re-enabled. The plan is to add an initial to local exec
relaxation as a replacement, though this has not been defined or
implemented yet.
This was tested with native gcc and glibc builds and checks with no
regressions.
Committed.
Jim
gcc/
* config/riscv/riscv.c (riscv_legitimize_tls_address): Ifdef out
use of TLS_MODEL_LOCAL_EXEC when not pic.
---
gcc/config/riscv/riscv.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c
index 3e0bedaf145..4ba811126fe 100644
--- a/gcc/config/riscv/riscv.c
+++ b/gcc/config/riscv/riscv.c
@@ -1257,9 +1257,12 @@ riscv_legitimize_tls_address (rtx loc)
rtx dest, tp, tmp;
enum tls_model model = SYMBOL_REF_TLS_MODEL (loc);
+#if 0
+ /* TLS copy relocs are now deprecated and should not be used. */
/* Since we support TLS copy relocs, non-PIC TLS accesses may all use LE. */
if (!flag_pic)
model = TLS_MODEL_LOCAL_EXEC;
+#endif
switch (model)
{
Thanks!
[PATCH] RISC-V: Implement __clear_cache via __builtin__clear_cache
We have had an implementation of __builtin__clear_cache since the beginning, but didn't have the cooresponding __clear_cache library routine implemented. This directly conflicts the GCC manual in a handful of places, which indicates that __clear_cache should work and that __builtin_clear_cache should function the same way as __clear_cache by ethier calling it or inlining the functionality. This patch simply implements __clear_cache via __builtin__clear_cache. This should be safe as we always have clear_cache insn so therefor __builtin__clear_cache will never fall back to calling __clear_cache. I'm not actually sure that silently implementing clear_cache as a NOP when there is no ISA defined mechanism for icache synchronization is the right way to go, but that's really a different discussion. This was reported as Bug 94136, which is a year old but was categorized as a documentation bug. I believe that categorization was incorrect: having an empty __clear_cache library routine is simply incorrect behavior, the fact that __builtin__clear_cache happens to be implemented as a libc call on Linux is just a red herring suggesting the documentation fix to point out the name difference. I view this new behavior as conforming to the existing documentation: we're just inlining the __clear_cache implementation, even if that implementation happens to be a call to a very similar looking libc routine. gcc/ChangeLog PR target/94136 * config/riscv/riscv.h (CLEAR_INSN_CACHE): New macro. --- Something has gone off the rails with my laptop and it's failing to build GCC, so I haven't actually tested this. I'm not sure it's sane to call a GCC builtin from within libgcc, but I figured it would be best to just send out the patch to ask. --- gcc/config/riscv/riscv.h | 5 + 1 file changed, 5 insertions(+) diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h index f3e85723c85..39a688ea1e9 100644 --- a/gcc/config/riscv/riscv.h +++ b/gcc/config/riscv/riscv.h @@ -993,4 +993,9 @@ extern void riscv_remove_unneeded_save_restore_calls (void); #define HARD_REGNO_RENAME_OK(FROM, TO) riscv_hard_regno_rename_ok (FROM, TO) +/* We always have a "clear_cache" insn, which means __builtin__clear_cache will + never emit a call to __clear_cache. */ +#undef CLEAR_INSN_CACHE +#define CLEAR_INSN_CACHE(BEG, END) __builtin__clear_cache((BEG), (END)); + #endif /* ! GCC_RISCV_H */ -- 2.31.1.527.g47e6f16901-goog
Re: [PATCH] Initialize a variable due to -Wmaybe-uninitialized.
On Mon, 18 Nov 2019 09:58:19 PST (-0800), l...@redhat.com wrote: On 11/18/19 6:17 AM, Martin Liška wrote: Hi. The patch is about yet another bootstrap -Wmaybe-uninitialized warning. I've just tested that on risv64 cross compiler with latest trunk. Ready to be installed? Thanks, Martin gcc/ChangeLog: 2019-11-18 Martin Liska PR bootstrap/92540 * config/riscv/riscv.c (riscv_address_insns): Initialize addr in order to remove boostrap -Wmaybe-uninitialized error. OK. I had this internally, but haven't had time to analyze if the warnings was a false positive or not. It's always initialized: the only path by which riscv_classify_address() doesn't initialize it is on an error, but riscv_classify_insns() handles that correctly -- well, at least without accessing an uninitialized variable, that implicit 3 is a bit ugly... Thanks for fixing this!
