diff mbox series

regalloc: Ignore '^' in early costing [PR114766]

Message ID PAWPR08MB898273ED5909CCC1026C1F4B831B2@PAWPR08MB8982.eurprd08.prod.outlook.com
State New
Headers show
Series regalloc: Ignore '^' in early costing [PR114766] | expand

Commit Message

Wilco Dijkstra April 29, 2024, 12:59 p.m. UTC 
According to documentation, '^' should only have an effect during reload.
However ira-costs.cc treats it in the same way as '?' during early costing.
As a result using '^' can accidentally disable valid alternatives and cause
significant regressions (see PR114741).  Avoid this by ignoring '^' during
costing.

Passes bootstrap and regress, OK for commit?

gcc:
        PR rtl-optimization/114766
        * ira-costs.cc (record_reg_classes): Ignore '^' during costing.

---

Comments

Vladimir Makarov April 30, 2024, 8:12 p.m. UTC | #1 
On 4/29/24 08:59, Wilco Dijkstra wrote:
> According to documentation, '^' should only have an effect during reload.
> However ira-costs.cc treats it in the same way as '?' during early costing.
> As a result using '^' can accidentally disable valid alternatives and cause
> significant regressions (see PR114741).  Avoid this by ignoring '^' during
> costing.
>
> Passes bootstrap and regress, OK for commit?
>
> gcc:
>          PR rtl-optimization/114766
>          * ira-costs.cc (record_reg_classes): Ignore '^' during costing.
>
Sorry, I can not accept this patch.  This constraint is used by other 
targets (as I know rs6000, s390, sh, gcn).  I suspect changing the 
semantics can affect the targets in some undesirable way.

Saying that, I understand that aarch64 needs the new semantics. So I 
think we can add a new hint with the required semantics. There are still 
few undefined characters for the new hint: '~', '-', '/', and '.' (may 
be I missed some),

I propose to use '~' for the new hint. So besides changing code, the 
documentation for '^' needs to clarified and documentation for code '~' 
should be added.

>
> diff --git a/gcc/ira-costs.cc b/gcc/ira-costs.cc
> index c86c5a16563aeefac9d4fa72839bee8d95409f4b..04d2f21b023f3456ba6f8c16c2418d7313965b2f 100644
> --- a/gcc/ira-costs.cc
> +++ b/gcc/ira-costs.cc
> @@ -771,10 +771,6 @@ record_reg_classes (int n_alts, int n_ops, rtx *ops,
>   		  c = *++p;
>   		  break;
>   
> -		case '^':
> -		  alt_cost += 2;
> -		  break;
> -
>   		case '?':
>   		  alt_cost += 2;
>   		  break;
>
>
>
diff mbox series

Patch

diff --git a/gcc/ira-costs.cc b/gcc/ira-costs.cc
index c86c5a16563aeefac9d4fa72839bee8d95409f4b..04d2f21b023f3456ba6f8c16c2418d7313965b2f 100644
--- a/gcc/ira-costs.cc
+++ b/gcc/ira-costs.cc
@@ -771,10 +771,6 @@  record_reg_classes (int n_alts, int n_ops, rtx *ops,
 		  c = *++p;
 		  break;
 
-		case '^':
-		  alt_cost += 2;
-		  break;
-
 		case '?':
 		  alt_cost += 2;
 		  break;