Re: [Bug jit/99126] Compilation ICE trying insert trap

[Andrea Corallo via Gcc-bugs]

"dmalcolm at gcc dot gnu.org via Gcc-bugs" 
writes:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99126
>
> --- Comment #8 from David Malcolm  ---
> (In reply to CVS Commits from comment #6)
>> The master branch has been updated by David Malcolm :
>> 
>> https://gcc.gnu.org/g:b258e263e0d74ca1f76aeaac5f4d1abef6b13707
>> 
>> commit r11-7288-gb258e263e0d74ca1f76aeaac5f4d1abef6b13707
>
> Fixed on trunk for gcc 11.  Andrea, do you need my to backport this?  What GCC
> versions are you targeting with your emacs project?

We are targetting them all, but to my test I could not trigger this bug
on GCC9 so I guess we could backport on GCC10 only?

Re: [Bug jit/99126] Compilation ICE trying insert trap

[Andrea Corallo via Gcc-bugs]

"dmalcolm at gcc dot gnu.org via Gcc-bugs" 
writes:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99126
>
> David Malcolm  changed:
>
>What|Removed |Added
> 
>  Status|NEW |ASSIGNED
>
> --- Comment #4 from David Malcolm  ---
> Am testing a fix.

Nice!

As a side question: do you guys think disabling "isolate-paths" is the
right workaround for the affected versions or might have harmful side
effects?

Thanks

  Andrea

Re: [Bug jit/99126] New: Compilation ICE trying insert trap

[Andrea Corallo via Gcc-bugs]

This is the bt of how the C front-end is initializing these
declarations:

#0  set_builtin_decl (implicit_p=, 
decl=, 
fncode=) at ../../gcc/tree.h:5662
#1  def_builtin_1 (fncode=, name=, 
fntype=, libtype=, both_p=, 
fallback_p=, nonansi_p=false, 
fnattrs=, implicit_p=true, 
fnclass=BUILT_IN_NORMAL)
at ../../gcc/c-family/c-common.c:4729
#2  0x00a291c9 in c_define_builtins (
va_list_arg_type_node=, va_list_ref_type_node=)
at ../../gcc/builtins.def:933

Thinking loud: I guess in jit-builtins.c we should loop once over all
the builtins calling 'set_builtin_decl'?  Probably in the constructor
for gcc::jit::builtins_manager?

Re: [Bug target/98931] [11 Regression] arm: Assembly fails with "branch out of range or not a multiple of 2" since r11-2012

[Andrea Corallo via Gcc-bugs]

Following suggestions I'm testing the attached emitting the following
for long branches where LE cannot cover:

subslr, #1
bmi .L2

>From 0cd38cb29829b48f96e8e060e7a875f49236b67b Mon Sep 17 00:00:00 2001
From: Andrea Corallo 
Date: Wed, 3 Feb 2021 15:21:54 +0100
Subject: [PATCH] arm: Add low overhead loop address range check [PR98931]

2021-02-05  Andrea Corallo  

* config/arm/thumb2.md: Generate alternative sequence for long
range branches.
---
 gcc/config/arm/thumb2.md | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/gcc/config/arm/thumb2.md b/gcc/config/arm/thumb2.md
index bd53bf320de..a8327066bfe 100644
--- a/gcc/config/arm/thumb2.md
+++ b/gcc/config/arm/thumb2.md
@@ -1719,7 +1719,18 @@
   (set (reg:SI LR_REGNUM)
(plus:SI (reg:SI LR_REGNUM) (const_int -1)))])]
   "TARGET_32BIT && TARGET_HAVE_LOB"
-  "le\t%|lr, %l0")
+  "*
+  if (get_attr_length (insn) == 2)
+return \"le\\t%|lr, %l0\";
+  else
+return \"subs\\t%|lr, #1\;bmi\\t%l0\";
+  "
+  [(set (attr "length")
+(if_then_else
+(lt (minus (pc) (match_dup 0)) (const_int 1024))
+   (const_int 2)
+   (const_int 6)))
+   (set_attr "type" "branch")])
 
 (define_expand "doloop_begin"
   [(match_operand 0 "" "")
-- 
2.20.1

Re: [Bug target/98931] [11 Regression] arm: Assembly fails with "branch out of range or not a multiple of 2" since r11-2012

[Andrea Corallo via Gcc-bugs]

Right I'll rework the patch.

Thanks

Re: [Bug target/98931] [11 Regression] arm: Assembly fails with "branch out of range or not a multiple of 2" since r11-2012

[Andrea Corallo via Gcc-bugs]

I'm testing the attached fix.

>From 6de0603f3a5c86396d44250cb34d4451528681b1 Mon Sep 17 00:00:00 2001
From: Andrea Corallo 
Date: Wed, 3 Feb 2021 15:21:54 +0100
Subject: [PATCH] arm: Add low overhead loop address range check [PR98931]

2021-02-03  Andrea Corallo  

* config/arm/arm.c (arm_target_insn_ok_for_lob): Add address range
check.
---
 gcc/config/arm/arm.c | 20 
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index e22396dbcd5..85c96cdb156 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -33769,10 +33769,22 @@ arm_target_insn_ok_for_lob (rtx insn)
  supported for 'low over head loop' making sure that LE target is
  above LE itself in the generated code.  */
 
-  return single_succ_p (bb)
-&& single_pred_p (bb)
-&& single_succ_edge (bb)->dest == single_pred_edge (bb)->src
-&& contains_no_active_insn_p (bb);
+  if  (!single_succ_p (bb)
+   || !single_pred_p (bb)
+   || single_succ_edge (bb)->dest != single_pred_edge (bb)->src
+   || !contains_no_active_insn_p (bb))
+return false;
+
+  unsigned int distance = 0;
+  rtx_insn *curr_insn = BB_HEAD (single_succ_edge (bb)->dest);
+  while ((curr_insn = NEXT_INSN (curr_insn)))
+distance += get_attr_length (curr_insn);
+
+  /* LE encodes the PC relative jump address in 12 bits.  */
+  if (distance >= (1 << 11))
+return false;
+
+  return true;
 }
 
 #if CHECKING_P
-- 
2.20.1

Re: [Bug target/98931] [11 Regression] arm: Assembly fails with "branch out of range or not a multiple of 2" since r11-2012

[Andrea Corallo via Gcc-bugs]

"jakub at gcc dot gnu.org via Gcc-bugs"  writes:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98931
>
> --- Comment #3 from Jakub Jelinek  ---
> And the problem is not something not being multiple of 2, but just out of 
> range
> jump.  The code has:
>   10:   f04e e001   dls lr, lr
>   14:   9900ldr r1, [sp, #0]
> ...
> 130e:   f00f c7ff   le  lr, 316 
> if I manually move .L2 label so that it doesn't complain anymore, which means
> it can jump only to -4096 bytes from the end of the le instruction, but it was
> supposed to jump to foo+0x14, i.e. to 4862 bytes before the end of the le insn
> at 0x1312.
> So, either the computation of the instruction sizes is incorrect, or for this
> kind of branch nothing checks whether the distance is in range.

I think this (the second) is the case.

Re: [Bug target/96372] [11 regression] arm/ivopts.c fails since r11-2012

[Andrea Corallo via Gcc-bugs]

I see what's going on, originally this object-size text <= 20 test was
gated with target 'arm_thumb2'.

My patch to exclude LOB targets replaced that with
'arm_thumb2_ok_no_arm_v8_1_lob', unfortunatelly this is relying on
'arm_thumb2_ok' that has a different semantic compared to the original
'arm_thumb2'.

I'll post the patch I'm testing.  Looks nothing serious to me tho.

  Andrea

Re: [Bug jit/98615] libgccjit crash while freeing 'clone_info' in 'cgraph_c_finalize'

[Andrea Corallo via Gcc-bugs]

"marxin at gcc dot gnu.org via Gcc-bugs"  writes:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98615
>
> --- Comment #8 from Martin Liška  ---
> (In reply to Andrea Corallo from comment #7)
>> Thanks Martin, I can confirm that also the bootstrap is back okay.
>
> You're welcome.
> Can you please point me to Emacs JIt usage? I'm curious what for is libgccjit
> used in the project?

That's a feature branch that being integrated as is planed to have it in
for 28.

Essentially we use it to compile .el files into shared libraries we then
load to have Elisp executed as native code.

Here is the recording for my presentation at LPC2020/Cauldron:


and this is my development blog:
.

  Andrea

Re: [Bug jit/98615] libgccjit crash while freeing 'clone_info' in 'cgraph_c_finalize'

[Andrea Corallo via Gcc-bugs]

Thanks Martin, I can confirm that also the bootstrap is back okay.

Re: [Bug jit/98615] libgccjit crash while freeing 'clone_info' in 'cgraph_c_finalize'

[Andrea Corallo via Gcc-bugs]

Thank you for looking into it!  I tried my self but with no success

Re: [Bug rtl-optimization/97092] [10/11 Regression] aarch64, SVE: ICE in ira-color.c since r10-4752-g2d56600c

[Andrea Corallo via Gcc-bugs]

"acoplan at gcc dot gnu.org via Gcc-bugs"  writes:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97092
>
> --- Comment #9 from Alex Coplan  ---
> Thanks for fixing this Andrea! FWIW I can reproduce the ICE with the same
> testcase and options on the head of the GCC 10 branch (contrary to my first
> message).

Hi Alex,

I'll test the patch also on gcc-10 then so we can have it there too (it
should apply).

Thanks for checking!

  Andrea

Re: [Bug rtl-optimization/97092] [10/11 Regression] aarch64, SVE: ICE in ira-color.c since r10-4752-g2d56600c

[Andrea Corallo via Gcc-bugs]

"rsandifo at gcc dot gnu.org via Gcc-bugs" 
writes:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97092
>
> --- Comment #6 from rsandifo at gcc dot gnu.org  
> ---
> (In reply to Andrea Corallo from comment #5)
>> "rsandifo at gcc dot gnu.org via Gcc-bugs" 
>> writes:
>> 
>> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97092
>> >
>> > --- Comment #4 from rsandifo at gcc dot gnu.org > > gnu.org> ---
>> > (In reply to Andrea Corallo from comment #3)
>> >> Created attachment 49710 [details]
>> >> PR97092.patch
>> >> 
>> >> What is going on is that in 'update_costs_from_allocno' we try to
>> >> identify the smallest mode using narrower_subreg_mode to then update the
>> >> costs.
>> >> 
>> >> The two modes involved here are E_DImode and E_VNx2QImode, cause these
>> >> are not ordered we ICE in 'paradoxical_subreg_p'.
>> >> 
>> >> Now I don't know if the strategy we want is:
>> >> 
>> >> - In 'update_costs_from_allocno' when modes are not ordered instead of
>> >>   calling 'narrower_subreg_mode' just keep the current one.
>> >> 
>> >> - Always select the cheapest mode in terms of cost.
>> >> 
>> >> The attached I'm testing implements the second.
>> 
>> Hi Richard,
>> 
>> thanks for commenting.
>> 
>> > I think instead we should consider recomputing “mode” in each
>> > iteration of the loop, rather than carry over the result of
>> > previous iterations.  I.e. use:
>> >
>> > mode = narrower_subreg_mode (ALLOCNO_MODE (cp->first),
>> >  ALLOCNO_MODE (cp->second));
>> 
>> Are we garanteed to have ALLOCNO_MODE (cp->first) and ALLOCNO_MODE
>> (cp->second) always satisfying 'ordered_p'?
> Yeah, I think so.  If the modes aren't ordered then we shouldn't
> create a copy between them.

Great, I'm going to test the suggested then.

Thanks

  Andrea

Re: [Bug rtl-optimization/97092] [10/11 Regression] aarch64, SVE: ICE in ira-color.c since r10-4752-g2d56600c

[Andrea Corallo via Gcc-bugs]

"rsandifo at gcc dot gnu.org via Gcc-bugs" 
writes:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97092
>
> --- Comment #4 from rsandifo at gcc dot gnu.org  
> ---
> (In reply to Andrea Corallo from comment #3)
>> Created attachment 49710 [details]
>> PR97092.patch
>> 
>> What is going on is that in 'update_costs_from_allocno' we try to
>> identify the smallest mode using narrower_subreg_mode to then update the
>> costs.
>> 
>> The two modes involved here are E_DImode and E_VNx2QImode, cause these
>> are not ordered we ICE in 'paradoxical_subreg_p'.
>> 
>> Now I don't know if the strategy we want is:
>> 
>> - In 'update_costs_from_allocno' when modes are not ordered instead of
>>   calling 'narrower_subreg_mode' just keep the current one.
>> 
>> - Always select the cheapest mode in terms of cost.
>> 
>> The attached I'm testing implements the second.

Hi Richard,

thanks for commenting.

> I think instead we should consider recomputing “mode” in each
> iteration of the loop, rather than carry over the result of
> previous iterations.  I.e. use:
>
> mode = narrower_subreg_mode (ALLOCNO_MODE (cp->first),
>  ALLOCNO_MODE (cp->second));

Are we garanteed to have ALLOCNO_MODE (cp->first) and ALLOCNO_MODE
(cp->second) always satisfying 'ordered_p'?  In case not I think we
can't use 'narrower_subreg_mode'.

I thought we select the smallest because is the cheapest, so not to use
'narrower_subreg_mode' I compared directly the costs.

> instead of:
>
> mode = narrower_subreg_mode (mode, ALLOCNO_MODE (cp->second));
>
> Before g:e2323a2b77c91d1ba8194b01e6deaa2e00f15990 “mode”
> was a loop invariant, so it made sense to set it outside
> the loop.  I think the intention of that patch was to use
> the smaller of the two modes involved in the copy, and carrying
> the result over to future copies might have been unintentional.
>
> The difficulty with carrying the mode over to later copies
> is that the costs then become dependent on the order of
> the copies, whereas I'm not sure the order of the copies
> is significant.

I see

Thanks!

  Andrea

Re: [Bug rtl-optimization/97092] [10/11 Regression] aarch64, SVE: ICE in ira-color.c since r10-4752-g2d56600c

[Andrea Corallo via Gcc-bugs]

What is going on is that in 'update_costs_from_allocno' we try to
identify the smallest mode using narrower_subreg_mode to then update the
costs.

The two modes involved here are E_DImode and E_VNx2QImode, cause these
are not ordered we ICE in 'paradoxical_subreg_p'.

Now I don't know if the strategy we want is:

- In 'update_costs_from_allocno' when modes are not ordered instead of
  calling 'narrower_subreg_mode' just keep the current one.

- Always select the cheapest mode in terms of cost.

The attached I'm testing implements the second.

  Andrea

>From 03da7bb5c7ad8ab6c25631de5ee1f7b70ea46229 Mon Sep 17 00:00:00 2001
From: Andrea Corallo 
Date: Tue, 8 Dec 2020 23:17:26 +0100
Subject: [PATCH] Fix ICE in ira-color

2020-12-08  Andrea Corallo  

* ira-color.c (update_costs_from_allocno): Pick the cheapest mode
instead of the smaller one.
---
 gcc/ira-color.c | 34 ++
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/gcc/ira-color.c b/gcc/ira-color.c
index d3f8e23faff..13a3548018a 100644
--- a/gcc/ira-color.c
+++ b/gcc/ira-color.c
@@ -1400,19 +1400,29 @@ update_costs_from_allocno (ira_allocno_t allocno, int 
hard_regno,
  || ALLOCNO_ASSIGNED_P (another_allocno))
continue;
 
- /* If we have different modes use the smallest one.  It is
-a sub-register move.  It is hard to predict what LRA
-will reload (the pseudo or its sub-register) but LRA
-will try to minimize the data movement.  Also for some
-register classes bigger modes might be invalid,
-e.g. DImode for AREG on x86.  For such cases the
-register move cost will be maximal.  */
- mode = narrower_subreg_mode (mode, ALLOCNO_MODE (cp->second));
+ /* If we have different modes use the cheapest one.  It is a
+sub-register move.  It is hard to predict what LRA will
+reload (the pseudo or its sub-register) but LRA will try
+to minimize the data movement.  Also for some register
+classes bigger modes might be invalid, e.g. DImode for
+AREG on x86.  For such cases the register move cost will
+be maximal.  */
+ machine_mode mode2 = ALLOCNO_MODE (cp->second);
  ira_init_register_move_cost_if_necessary (mode);
- 
- cost = (cp->second == allocno
- ? ira_register_move_cost[mode][rclass][aclass]
- : ira_register_move_cost[mode][aclass][rclass]);
+ ira_init_register_move_cost_if_necessary (mode2);
+ int cost1, cost2;
+ if (cp->second == allocno)
+   {
+ cost1 = ira_register_move_cost[mode][rclass][aclass];
+ cost2 = ira_register_move_cost[mode2][rclass][aclass];
+   }
+ else
+   {
+ cost1 = ira_register_move_cost[mode][aclass][rclass];
+ cost2 = ira_register_move_cost[mode2][aclass][rclass];
+   }
+ cost = MIN (cost1, cost2);
+
  if (decr_p)
cost = -cost;
 
-- 
2.20.1

Re: [Bug rtl-optimization/97092] [10/11 Regression] aarch64, SVE: ICE in ira-color.c since r10-4752-g2d56600c

[Andrea Corallo via Gcc-bugs]

"akrl at gcc dot gnu.org via Gcc-bugs"  writes:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97092
>
> --- Comment #1 from akrl at gcc dot gnu.org ---
> Hi all,
>
> I can't reproduce this on current master (76a1719f0ff), I guess has been 
> fixed in the meanwhile?

As not said I swapped two reproducers.  I *can* reproduce the issue on
current master.

  Andrea