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

[Alan Lawrence <alan dot lawrence at arm dot com>]

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!)

--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;
diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index d1b59a1efd81ee46910e7971a2f0cd1b6e3e10b0..8c2b83614f98675b0d55e5e724e6d9c9e0c9b909 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -6647,8 +6647,8 @@ tree_swap_operands_p (const_tree arg0, const_tree arg1, bool reorder)
   if (CONSTANT_CLASS_P (arg0))
     return 1;
 
-  STRIP_NOPS (arg0);
-  STRIP_NOPS (arg1);
+  STRIP_SIGN_NOPS (arg0);
+  STRIP_SIGN_NOPS (arg1);
 
   if (TREE_CONSTANT (arg1))
     return 0;