Re: Re: [PATCH 2/4] df: Add DF_LIVE_SUBREG problem

[钟居哲 <juzhe.zhong@rivai.ai>]

[-- Attachment #1: Type: text/plain, Size: 2360 bytes --]

Thanks Dim.

We noticed there is regression in aarch64 CI.
We will fix it with following your comments and regression in aarch64 CI.



juzhe.zhong@rivai.ai
 
From: Dimitar Dimitrov
Date: 2024-05-08 23:57
To: 陈硕
CC: 丁乐华; gcc-patches; 钟居哲; 夏晋; vmakarov; richard.sandiford
Subject: Re: [PATCH 2/4] df: Add DF_LIVE_SUBREG problem
On Wed, May 08, 2024 at 11:34:48AM +0800, 陈硕 wrote:
> Hi Dimitar
> 
> 
> I send a patch just now, modifies accordingly
> 
> 
> some comments:
> 
> 
> >Nit: Should have two spaces after the dot, per GNU coding style.  I'd suggest
> >to run the contrib/check_GNU_style.py script on your patches.
> Do you mean "star" by "dot", i.e. "/*----" should be "/* ----"?
 
No, I was referring to the following paragraph from
https://www.gnu.org/prep/standards/standards.html :
   "Please put two spaces after the end of a sentence in your comments, ..."
 
To fix, simply add a second space after the dot, e.g.:
  -   Like DF_LR, but include tracking subreg liveness. Currently used to provide
  +   Like DF_LR, but include tracking subreg liveness.  Currently used to provide
 
 
For reference, here is the output from the style checker:
  $ git show | ./contrib/check_GNU_style.py -
  === ERROR type #4: dot, space, space, new sentence (24 error(s)) ===
  ...
  gcc/df-problems.cc:1350:52:   Like DF_LR, but include tracking subreg liveness.█Currently used to provide
 
> 
> 
> >These names seem a bit too short for global variables.  Perhaps tuck
> >them in a namespace?
> >
> >Also, since these must remain empty, shouldn't they be declared as const?
> >
> >namespace df {
> >   const bitmap_head empty_bitmap;
> >   const subregs_live empty_live;
> >}
> 
> 
> 
> May be better if "namespace df" contains all DF related code? as a minor modification, I add a prefix "df_" to the variables.
> Meanwhile, const seems inapropriate here, since it's returned as normal pointer rather than const pointer in some funtions, 
> 
> change to const would break this return value type check, and a const_cast would make the const meanlingless.
> 
> 
> more details see in the patch
 
Thanks for considering my suggestion.
 
Regards,
Dimitar
> 
> 
> regards
> Shuo
> 
>  
>