Request for 8.2, SVE cores && adding SVE to NEWS file etc

Request for 8.2, SVE cores && adding SVE to NEWS file etc

[Alan Hayward]

Wasn’t sure if this was the best place to ask.

1) Is 8.2 still open to new functionality? It’d be really useful to have core support for Aarch64 SVE in 8.2
(Given SVE is supported in 8.2, I guess you could argue it’s a bug fix :). )
Currently in 8.2, GDB will just ignore the SVE parts of a core file.

The four commits are:
(Earliest patch is obv, second is a mechanical change across many files, the final two are aarch64 only)

0c76e06d5c   2 days ago Alan Hayward    Parse SVE registers in aarch64 core file reading/writing
b7fd65b9dc   2 days ago Alan Hayward    Detect SVE when reading aarch64 core files
a616bb9450   2 days ago Alan Hayward    Split size in regset section iterators
dbd534fee4   5 days ago Alan Hayward    Rename size in get_core_register_sectio


2) I’ve spotted that Aarch64 SVE support should probably be mentioned in the NEWS file.
Are there any other documentation type files that need updating alongside?


Thanks,
Alan.

Re: Request for 8.2, SVE cores && adding SVE to NEWS file etc

[Pedro Alves]

On 08/15/2018 10:31 AM, Alan Hayward wrote:
> Wasn’t sure if this was the best place to ask.
> 
> 1) Is 8.2 still open to new functionality? It’d be really useful to have core support for Aarch64 SVE in 8.2
> (Given SVE is supported in 8.2, I guess you could argue it’s a bug fix :). )
> Currently in 8.2, GDB will just ignore the SVE parts of a core file.

Not usually, but there have been exceptions evaluated on a case by case basis.

> 
> The four commits are:
> (Earliest patch is obv, second is a mechanical change across many files, the final two are aarch64 only)
> 
> 0c76e06d5c   2 days ago Alan Hayward    Parse SVE registers in aarch64 core file reading/writing
> b7fd65b9dc   2 days ago Alan Hayward    Detect SVE when reading aarch64 core files
> a616bb9450   2 days ago Alan Hayward    Split size in regset section iterators
> dbd534fee4   5 days ago Alan Hayward    Rename size in get_core_register_sectio

I haven't looked at the patches, but in principle, I'd support it,
from the standpoint that since we're adding SVE support, might as well
add all of it.  Also, speaking with my Fedora/RHEL had on, we'd likely end
up backporting them too.  Can you give a guesstimate of the risk those
patches bring in to other ports?  I'd assume they'd be pretty self contained.

> 
> 
> 2) I’ve spotted that Aarch64 SVE support should probably be mentioned in the NEWS file.
> Are there any other documentation type files that need updating alongside?

Well, the user manual, if there were any changes to it.  

If the SVE support included any new XML target description feature,
that should be mentioned in the manual.

Thanks,
Pedro Alves

Re: Request for 8.2, SVE cores && adding SVE to NEWS file etc

[Alan Hayward]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3192 bytes --]



> On 16 Aug 2018, at 17:17, Pedro Alves <palves@redhat.com> wrote:
> 
> On 08/15/2018 10:31 AM, Alan Hayward wrote:
>> Wasn’t sure if this was the best place to ask.
>> 
>> 1) Is 8.2 still open to new functionality? It’d be really useful to have core support for Aarch64 SVE in 8.2
>> (Given SVE is supported in 8.2, I guess you could argue it’s a bug fix :). )
>> Currently in 8.2, GDB will just ignore the SVE parts of a core file.
> 
> Not usually, but there have been exceptions evaluated on a case by case basis.
> 
>> 
>> The four commits are:
>> (Earliest patch is obv, second is a mechanical change across many files, the final two are aarch64 only)
>> 
>> 0c76e06d5c   2 days ago Alan Hayward    Parse SVE registers in aarch64 core file reading/writing
>> b7fd65b9dc   2 days ago Alan Hayward    Detect SVE when reading aarch64 core files
>> a616bb9450   2 days ago Alan Hayward    Split size in regset section iterators
>> dbd534fee4   5 days ago Alan Hayward    Rename size in get_core_register_sectio
> 
> I haven't looked at the patches, but in principle, I'd support it,
> from the standpoint that since we're adding SVE support, might as well
> add all of it.  Also, speaking with my Fedora/RHEL had on, we'd likely end
> up backporting them too.  Can you give a guesstimate of the risk those
> patches bring in to other ports?  I'd assume they'd be pretty self contained.

There’s a simple enough change to the args of a common function. And then everything
else is self contained. I’d break it down into:

dbd534fee4   5 days ago Alan Hayward    Rename size in get_core_register_sectio
 - Rename of single variable in common function. Obvious change, no risk.

a616bb9450   2 days ago Alan Hayward    Split size in regset section iterators
 - Mechanical change across every tdep file (adds a new third arg, which is a
   duplicate of the second arg). Very minor risk on any target that there might
   be a copy/paste error that hasn't been spotted (just needs a triple check of
   the diff again when merging). Minor risk that I've missed a tdep file that
   doesn't get built into a target-all build (causing a build error which I’d
   hope would have been spotted by now).

b7fd65b9dc   2 days ago Alan Hayward    Detect SVE when reading aarch64 core files
0c76e06d5c   2 days ago Alan Hayward    Parse SVE registers in aarch64 core file reading/writing
 - Limited to reading/writing Aarch64 (SVE and non-SVE) Linux cores on either
   a Aarch64 Linux build or target-all build. SVE code is seperated from
   standard aarch64 code. So to go wrong on non-SVE, the code would first
   have to incorrectly detect the presence of SVE.

> 
>> 
>> 
>> 2) I’ve spotted that Aarch64 SVE support should probably be mentioned in the NEWS file.
>> Are there any other documentation type files that need updating alongside?
> 
> Well, the user manual, if there were any changes to it.  
> 
> If the SVE support included any new XML target description feature,
> that should be mentioned in the manual.

Ok, I’ll put together a patch for both of those.

Thanks,
Alan.


\x16º&Öéj×!zÊÞ¶êç׏|ïyb²Ö«r\x18\x1dn–­r\x17¬

Re: Request for 8.2, SVE cores && adding SVE to NEWS file etc

[Joel Brobecker]

> There’s a simple enough change to the args of a common function. And then everything
> else is self contained. I’d break it down into:
> 
> dbd534fee4   5 days ago Alan Hayward    Rename size in get_core_register_sectio
>  - Rename of single variable in common function. Obvious change, no risk.
> 
> a616bb9450   2 days ago Alan Hayward    Split size in regset section iterators
>  - Mechanical change across every tdep file (adds a new third arg, which is a
>    duplicate of the second arg). Very minor risk on any target that there might
>    be a copy/paste error that hasn't been spotted (just needs a triple check of
>    the diff again when merging). Minor risk that I've missed a tdep file that
>    doesn't get built into a target-all build (causing a build error which I’d
>    hope would have been spotted by now).
> 
> b7fd65b9dc   2 days ago Alan Hayward    Detect SVE when reading aarch64 core files
> 0c76e06d5c   2 days ago Alan Hayward    Parse SVE registers in aarch64 core file reading/writing
>  - Limited to reading/writing Aarch64 (SVE and non-SVE) Linux cores on either
>    a Aarch64 Linux build or target-all build. SVE code is seperated from
>    standard aarch64 code. So to go wrong on non-SVE, the code would first
>    have to incorrectly detect the presence of SVE.

I took a look at the patches, and I agree they look sufficiently
self-contained that they should be OK for inclusion in 8.2. Give
Pedro a few days to comment as well, just in case, but otherwise,
it is OK for me to backport.

-- 
Joel

Re: Request for 8.2, SVE cores && adding SVE to NEWS file etc

[Pedro Alves]

On 08/21/2018 02:16 PM, Joel Brobecker wrote:
>> There’s a simple enough change to the args of a common function. And then everything
>> else is self contained. I’d break it down into:
>>
>> dbd534fee4   5 days ago Alan Hayward    Rename size in get_core_register_sectio
>>  - Rename of single variable in common function. Obvious change, no risk.
>>
>> a616bb9450   2 days ago Alan Hayward    Split size in regset section iterators
>>  - Mechanical change across every tdep file (adds a new third arg, which is a
>>    duplicate of the second arg). Very minor risk on any target that there might
>>    be a copy/paste error that hasn't been spotted (just needs a triple check of
>>    the diff again when merging). Minor risk that I've missed a tdep file that
>>    doesn't get built into a target-all build (causing a build error which I’d
>>    hope would have been spotted by now).
>>
>> b7fd65b9dc   2 days ago Alan Hayward    Detect SVE when reading aarch64 core files
>> 0c76e06d5c   2 days ago Alan Hayward    Parse SVE registers in aarch64 core file reading/writing
>>  - Limited to reading/writing Aarch64 (SVE and non-SVE) Linux cores on either
>>    a Aarch64 Linux build or target-all build. SVE code is seperated from
>>    standard aarch64 code. So to go wrong on non-SVE, the code would first
>>    have to incorrectly detect the presence of SVE.
> 
> I took a look at the patches, and I agree they look sufficiently
> self-contained that they should be OK for inclusion in 8.2. Give
> Pedro a few days to comment as well, just in case, but otherwise,
> it is OK for me to backport.

I have no further comments.  Fine with me to go ahead.

Thanks,
Pedro Alves