This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Put all constants last in tree_swap_operands_p, remove odd -Os check


s/tiny/small/ and I think we might be getting close. So the 403.gcc test being compiled contains:

if (regno == PIC_OFFSET_TABLE_REGNUM
          && fixed_regs[PIC_OFFSET_TABLE_REGNUM])
        return pic_offset_table_rtx;

PIC_OFFSET_TABLE_REGNUM is an expression testing machine flags; prior to Richie's patch, it looks like constant propagation/CSE/forward propagation wasn't really figuring this out, so was computing PIC_OFFSET_TABLE_REGNUM into a register, and adding this to (fixed_regs + 0, supplied by the linker).

Following Richie's patch, PIC_OFFSET_TABLE_REGNUM resolves to INVALID_REGNUM = (~(unsigned int) 0), i.e. the ffffffff in the relocations. We then ask the linker for (fixed_regs + ffffffff), but of course, if (fixed_regs > 0) then that doesn't fit into the 32 bits allowed by the small memory model.

In neither case is the index into fixed_regs actually ever executed, of course!

So yes, my workaround is wrong, we are working on a proper fix...

--Alan
	

Andrew Pinski wrote:
On Mon, Sep 22, 2014 at 4:10 AM, Alan Lawrence <alan.lawrence@arm.com> wrote:
Well, I haven't looked into this in detail: I've gone only as far as
  * swapping emit-rtl.o between 'good' compiles (svn r214042) and 'bad'
compiles (r214043), finding that the critical difference is in the
emit-rtl.o generated by r214043;
  *looking at the relocations in the 'bad' emit_rtl.o, seeing new entries
'fixed_regs + ffffffff', and that Richard Biener's changelog specifically
mentions stripping signedness changes (and introduces the SIGN_NOPS).

However, I apply your patch (minus the hunk adding the (set_attr "type"
load1"), this appears to have gone in already), and still see the same error
message:

emit-rtl.o: In function `gen_rtx_REG':
emit-rtl.c:(.text+0x12f8): relocation truncated to fit:
R_AARCH64_ADR_PREL_PG_HI21 against symbol `fixed_regs' defined in COMMON
section in regclass.o
emit-rtl.o: In function `gen_rtx':
emit-rtl.c:(.text+0x1824): relocation truncated to fit:
R_AARCH64_ADR_PREL_PG_HI21 against symbol `fixed_regs' defined in COMMON
section in regclass.o
collect2: error: ld returned 1 exit status

and still see the same (suspicious-looking, although perhaps not convicted)
relocations:

$ readelf --relocs
benchspec/CPU2006/403.gcc/build/build_base_test.0000/emit-rtl.o | grep
fixed_regs
0000000012a8  005d00000113 R_AARCH64_ADR_PRE 0000000000000000 fixed_regs + 0
0000000012ac  005d00000115 R_AARCH64_ADD_ABS 0000000000000000 fixed_regs + 0
0000000012f8  005d00000113 R_AARCH64_ADR_PRE 0000000000000000 fixed_regs +
ffffffff
0000000012fc  005d00000116 R_AARCH64_LDST8_A 0000000000000000 fixed_regs +
ffffffff
000000001824  005d00000113 R_AARCH64_ADR_PRE 0000000000000000 fixed_regs +
ffffffff
000000001828  005d00000116 R_AARCH64_LDST8_A 0000000000000000 fixed_regs +
ffffffff
00000000186c  005d00000113 R_AARCH64_ADR_PRE 0000000000000000 fixed_regs + 0
000000001870  005d00000115 R_AARCH64_ADD_ABS 0000000000000000 fixed_regs + 0

I've also now bootstrapped my patch (STRIP_NOPS -> STRIP_SIGN_NOPS * 2) on
aarch64-none-linux-gnu and x86_64-none-linux-gnu, and check-gcc with no
regressions, so would like to propose that patch for trunk...?


You need to track down where R_AARCH64_ADR_PREL_PG_HI21 reloc is being
created in the assembly and then track down why GCC is using tiny
model here.  Note my fix was for a similar issue; not necessary the
exact same one in that there could be another pattern which needs to
use the new constraint too.

Thanks,
Andrew

--Alan




Andrew Pinski wrote:
On Thu, Sep 18, 2014 at 9:44 AM, Alan Lawrence <alan.lawrence@arm.com>
wrote:
We've been seeing errors using aarch64-none-linux-gnu gcc to build the
403.gcc benchmark from spec2k6, that we've traced back to this patch. The
error looks like:

/home/alalaw01/bootstrap_richie/gcc/xgcc
-B/home/alalaw01/bootstrap_richie/gcc -O3 -mcpu=cortex-a57.cortex-a53
-DSPEC_CPU_LP64        alloca.o asprintf.o vasprintf.o c-parse.o c-lang.o
attribs.o c-errors.o c-lex.o c-pragma.o c-decl.o c-typeck.o c-convert.o
c-aux-info.o c-common.o c-format.o c-semantics.o c-objc-common.o main.o
cpplib.o cpplex.o cppmacro.o cppexp.o cppfiles.o cpphash.o cpperror.o
cppinit.o cppdefault.o line-map.o mkdeps.o prefix.o version.o mbchar.o
alias.o bb-reorder.o bitmap.o builtins.o caller-save.o calls.o cfg.o
cfganal.o cfgbuild.o cfgcleanup.o cfglayout.o cfgloop.o cfgrtl.o
combine.o
conflict.o convert.o cse.o cselib.o dbxout.o debug.o dependence.o df.o
diagnostic.o doloop.o dominance.o dwarf2asm.o dwarf2out.o dwarfout.o
emit-rtl.o except.o explow.o expmed.o expr.o final.o flow.o fold-const.o
function.o gcse.o genrtl.o ggc-common.o global.o graph.o haifa-sched.o
hash.o hashtable.o hooks.o ifcvt.o insn-attrtab.o insn-emit.o
insn-extract.o
insn-opinit.o insn-output.o insn-peep.o insn-recog.o integrate.o intl.o
jump.o langhooks.o lcm.o lists.o local-alloc.o loop.o obstack.o optabs.o
params.o predict.o print-rtl.o print-tree.o profile.o real.o recog.o
reg-stack.o regclass.o regmove.o regrename.o reload.o reload1.o reorg.o
resource.o rtl.o rtlanal.o rtl-error.o sbitmap.o sched-deps.o sched-ebb.o
sched-rgn.o sched-vis.o sdbout.o sibcall.o simplify-rtx.o ssa.o ssa-ccp.o
ssa-dce.o stmt.o stor-layout.o stringpool.o timevar.o toplev.o tree.o
tree-dump.o tree-inline.o unroll.o varasm.o varray.o vmsdbgout.o
xcoffout.o
ggc-page.o i386.o xmalloc.o xexit.o hashtab.o safe-ctype.o splay-tree.o
xstrdup.o md5.o fibheap.o xstrerror.o concat.o partition.o hex.o
lbasename.o
getpwd.o ucbqsort.o             -lm        -o gcc
emit-rtl.o: In function `gen_rtx_REG':
emit-rtl.c:(.text+0x12f8): relocation truncated to fit:
R_AARCH64_ADR_PREL_PG_HI21 against symbol `fixed_regs' defined in COMMON
section in regclass.o
emit-rtl.o: In function `gen_rtx':
emit-rtl.c:(.text+0x1824): relocation truncated to fit:
R_AARCH64_ADR_PREL_PG_HI21 against symbol `fixed_regs' defined in COMMON
section in regclass.o
collect2: error: ld returned 1 exit status
specmake: *** [gcc] Error 1
Error with make 'specmake -j7 build': check file

'/home/alalaw01/spectest/benchspec/CPU2006/403.gcc/build/build_base_test.0000/make.err'
  Command returned exit code 2
  Error with make!
*** Error building 403.gcc

Inspecting the compiled emit-rtl.o shows:

$ readelf --relocs good/emit-rtl.o | grep fixed_regs
0000000012a8 005d00000113 R_AARCH64_ADR_PRE 0000000000000000 fixed_regs +
0
0000000012ac 005d00000115 R_AARCH64_ADD_ABS 0000000000000000 fixed_regs +
0
000000001800 005d00000113 R_AARCH64_ADR_PRE 0000000000000000 fixed_regs +
0
000000001804 005d00000115 R_AARCH64_ADD_ABS 0000000000000000 fixed_regs +
0

(that's compiled with a gcc just before this patch), contrastingly using
a
gcc with that patch:

$ readelf --relocs bad/emit-rtl.o | grep fixed_regs
0000000012a8 005d00000113 R_AARCH64_ADR_PRE 0000000000000000 fixed_regs +
0
0000000012ac 005d00000115 R_AARCH64_ADD_ABS 0000000000000000 fixed_regs +
0
0000000012f8 005d00000113 R_AARCH64_ADR_PRE 0000000000000000 fixed_regs +
ffffffff
0000000012fc 005d00000116 R_AARCH64_LDST8_A 0000000000000000 fixed_regs +
ffffffff
000000001824 005d00000113 R_AARCH64_ADR_PRE 0000000000000000 fixed_regs +
ffffffff
000000001828 005d00000116 R_AARCH64_LDST8_A 0000000000000000 fixed_regs +
ffffffff
00000000186c 005d00000113 R_AARCH64_ADR_PRE 0000000000000000 fixed_regs +
0
000000001870 005d00000115 R_AARCH64_ADD_ABS 0000000000000000 fixed_regs +
0

I attach a candidate 'fix', which allows building of 403.gcc on
aarch64-none-linux-gnu, full regression etc ongoing. (I admit there may
be
better options in terms of canonicalizing if you want to!)


I don't think this is the correct fix or even close to the real issue.
I think we have some tiny memory model issues coming in when medium
memory model is being used.  I ran into this issue while compiling php
with a GCC 4.7 based compiler.  Try the attached patch.

Thanks,
Andrew Pinski

--Alan



Richard Biener wrote:
The following makes tree_swap_operands_p put all constants 2nd place,
also looks through sign-changes when considering further
canonicalzations
and removes the odd -Os guard for those.  That was put in with
https://gcc.gnu.org/ml/gcc-patches/2003-10/msg01208.html just
motivated by CSiBE numbers - but rather than disabling canonicalization
this should have disabled the actual harmful transforms.

Bootstrap and regtest ongoing on x86_64-unknown-linux-gnu.

Richard.

2014-08-15  Richard Biener  <rguenther@suse.de>

        * fold-const.c (tree_swap_operands_p): Put all constants
        last, also strip sign-changing NOPs when considering further
        canonicalization.  Canonicalize also when optimizing for size.

Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c    (revision 214007)
+++ gcc/fold-const.c    (working copy)
@@ -6642,37 +6650,19 @@ reorder_operands_p (const_tree arg0, con
 bool
 tree_swap_operands_p (const_tree arg0, const_tree arg1, bool reorder)
 {
-  STRIP_SIGN_NOPS (arg0);
-  STRIP_SIGN_NOPS (arg1);
-
-  if (TREE_CODE (arg1) == INTEGER_CST)
+  if (CONSTANT_CLASS_P (arg1) == INTEGER_CST)
     return 0;
-  if (TREE_CODE (arg0) == INTEGER_CST)
+  if (CONSTANT_CLASS_P (arg0) == INTEGER_CST)
     return 1;
 -  if (TREE_CODE (arg1) == REAL_CST)
-    return 0;
-  if (TREE_CODE (arg0) == REAL_CST)
-    return 1;
-
-  if (TREE_CODE (arg1) == FIXED_CST)
-    return 0;
-  if (TREE_CODE (arg0) == FIXED_CST)
-    return 1;
-
-  if (TREE_CODE (arg1) == COMPLEX_CST)
-    return 0;
-  if (TREE_CODE (arg0) == COMPLEX_CST)
-    return 1;
+  STRIP_NOPS (arg0);
+  STRIP_NOPS (arg1);
    if (TREE_CONSTANT (arg1))
     return 0;
   if (TREE_CONSTANT (arg0))
     return 1;
 -  if (optimize_function_for_size_p (cfun))
-    return 0;
-
   if (reorder && flag_evaluation_order
       && (TREE_SIDE_EFFECTS (arg0) || TREE_SIDE_EFFECTS (arg1)))
     return 0;







Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]