| Message ID | 1451339699-7531-1-git-send-email-slyfox@inbox.ru |
|---|---|
| State | New |
| Headers | show |
On 12/28/2015 02:54 PM, Sergei Trofimovich wrote: > From: Sergei Trofimovich <siarheit@google.com> > > Tested on the following example: > > void * a[77] __attribute((visibility("hidden"))); > void f(long o, void * v) { a[0x6ffffeff - o + 66] = v; } > > Before the patch generated code uses .GOT entry: > > addl r14 = @ltoffx(a#), r1 > ld8.mov r14 = [r14], a# > > After the patch generated code uses static gprel relocation: > > movl r14 = @gprel(a#) > add r14 = r1, r14 > > That way gcc will be able to compile glibc's ld: PR/60465 Egad. PIC on ia64 is a mess. I can kind of see what Richard was trying to do, but ewww. I don't think it's worth the effort to deep dive into the PIC support and make ia64 handle things like most other ports -- it's a dead architecture so ISTM the easiest fix is the right fix. A few, relatively minor things. > > Bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60465 > Signed-off-by: Sergei Trofimovich <siarheit@google.com> > --- > gcc/config/ia64/ia64.c | 2 ++ > gcc/config/ia64/predicates.md | 26 ++++++++++++++++++++++++++ > 2 files changed, 28 insertions(+) > > diff --git a/gcc/config/ia64/ia64.c b/gcc/config/ia64/ia64.c > index f48cebc..6ea5072 100644 > --- a/gcc/config/ia64/ia64.c > +++ b/gcc/config/ia64/ia64.c > @@ -1105,6 +1105,8 @@ ia64_expand_load_address (rtx dest, rtx src) > emit_insn (gen_load_fptr (dest, src)); > else if (sdata_symbolic_operand (src, VOIDmode)) > emit_insn (gen_load_gprel (dest, src)); > + else if (local_symbolic_operand64 (src, VOIDmode)) > + emit_insn (gen_load_gprel64 (dest, src)); Comment here. Something like /* We want to use gprel rather than ltoff relocations for local symbolic operands. */ > > +;; True if OP refers to a local symbol +any large offset). ;; True if OP refers to a local symbol [+ any offset ] I haven't dug into the ia64 port (and I'm not planning to) to see if/how it MINUS in symbolic expressions. It's been the source of problems in various ports trough the years. Can you take the testcase from your post as well as the one from BZ60465 comment #37 (from you) and add them to the testsuite? You can dump them into testsuite/gcc.target/ia64. I think you'll just want to scan the assembler output to verify you're not getting ltoff relocations. Look for uses of scan-assembler-not in other tests for examples. Normally we'd require a bootstrap & regression test. We're more lenient with patches to dead architectures. What I'd do is something like Add new tests to <path_to_sources/gcc/testsuite/gcc.target/ia64/ <path_to_sources>/configure <any flags you wanted> make <-j whatever> all-gcc cd gcc make check-gcc RUNTESTFLAGS=ia64.exp Save those results. Ideally everything will pass except your two new tests. Apply your patch to ia64.c/predicates.md then just make <-j whatever> all-gcc cd gcc make check-gcc RUNTESTFLAGS=ia64.exp Note you're not running the full testsuite, just a few dozen ia64 specific tests, which will include your new tests. ANd you're not rebuilding the whole compiler between those steps, just ia64.o and relinking the compiler. So it ought to be reasonably fast. So to summarize, I think your patch needs the two trivial comment fixes noted above, 2 testcases and the before/after results of running just the ia64.exp tests. Repost with that and I'll get it into the tree. Jeff
> Normally we'd require a bootstrap & regression test. We're more lenient > with patches to dead architectures. I can do it if Sergei or others cannot though.
diff --git a/gcc/config/ia64/ia64.c b/gcc/config/ia64/ia64.c index f48cebc..6ea5072 100644 --- a/gcc/config/ia64/ia64.c +++ b/gcc/config/ia64/ia64.c @@ -1105,6 +1105,8 @@ ia64_expand_load_address (rtx dest, rtx src) emit_insn (gen_load_fptr (dest, src)); else if (sdata_symbolic_operand (src, VOIDmode)) emit_insn (gen_load_gprel (dest, src)); + else if (local_symbolic_operand64 (src, VOIDmode)) + emit_insn (gen_load_gprel64 (dest, src)); else { HOST_WIDE_INT addend = 0; diff --git a/gcc/config/ia64/predicates.md b/gcc/config/ia64/predicates.md index 2aa7a78..9c6951d 100644 --- a/gcc/config/ia64/predicates.md +++ b/gcc/config/ia64/predicates.md @@ -97,6 +97,32 @@ } }) +;; True if OP refers to a local symbol +any large offset). +;; To be encoded as: +;; movl % = @gprel(symbol+offset) +;; add % = %, gp +(define_predicate "local_symbolic_operand64" + (match_code "symbol_ref,const") +{ + switch (GET_CODE (op)) + { + case CONST: + op = XEXP (op, 0); + if (GET_CODE (op) != PLUS + || GET_CODE (XEXP (op, 0)) != SYMBOL_REF + || GET_CODE (XEXP (op, 1)) != CONST_INT) + return false; + op = XEXP (op, 0); + /* FALLTHRU */ + + case SYMBOL_REF: + return SYMBOL_REF_LOCAL_P (op); + + default: + gcc_unreachable (); + } +}) + ;; True if OP refers to a symbol in the small address area. (define_predicate "small_addr_symbolic_operand" (match_code "symbol_ref,const")