Bug 400490 - s390x: VRs allocated as if separate from FPRs
Summary: s390x: VRs allocated as if separate from FPRs
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: vex (other bugs)
Version First Reported In: 3.14 SVN
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Andreas Arnez
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-10-30 18:06 UTC by Andreas Arnez
Modified: 2018-11-14 15:32 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed/Implemented In:
Sentry Crash Report:


Attachments
Fix register allocation for VRs vs FPRs (3.06 KB, patch)
2018-10-30 18:47 UTC, Andreas Arnez
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Arnez 2018-10-30 18:06:39 UTC
On s390x, if vector registers are available, they are fed to the register allocator as if they were separate from the floating-point registers.  But in fact the FPRs are embedded in the VRs.  So for instance, if both f3 and v3 are allocated and used at the same time, corruption will result.
Comment 1 Andreas Arnez 2018-10-30 18:47:50 UTC
Created attachment 115990 [details]
Fix register allocation for VRs vs FPRs

This patch fixes the issue by offering only the non-overlapping vector registers to the register allocator.
Comment 2 Julian Seward 2018-11-09 09:40:09 UTC
(In reply to Andreas Arnez from comment #1)
> Created attachment 115990 [details]
> Fix register allocation for VRs vs FPRs
> 
> This patch fixes the issue by offering only the non-overlapping vector
> registers to the register allocator.

OK to land.  This seems perfectly reasonable and is the same solution
that has been used for some other targets -- 32-bit ARM and possibly
other targets (Power, I think).

+   RRegUniverse__check_is_sane(ru);

Good!  That wasn't there before?!

I assume this fix would be a candidate to ship in a 3.14.1 if we do
such a release .. is that correct?  In other words, does this bug
affect 3.14.0 ?
Comment 3 Andreas Arnez 2018-11-14 15:32:55 UTC
(In reply to Julian Seward from comment #2)
> OK to land.  This seems perfectly reasonable and is the same solution
> that has been used for some other targets -- 32-bit ARM and possibly
> other targets (Power, I think).
Thanks, so I'll leave it at that.

> 
> +   RRegUniverse__check_is_sane(ru);
> 
> Good!  That wasn't there before?!
Yeah...

> I assume this fix would be a candidate to ship in a 3.14.1 if we do
> such a release .. is that correct?  In other words, does this bug
> affect 3.14.0 ?
Yes, I actually ran into this, so the fix would certainly be a candidate for 3.14.1.

Pushed as git commit 71002d8a5.