Bug 400490

Summary: s390x: VRs allocated as if separate from FPRs
Product: [Developer tools] valgrind Reporter: Andreas Arnez <arnez>
Component: vexAssignee: Andreas Arnez <arnez>
Status: RESOLVED FIXED    
Severity: normal CC: jseward
Priority: NOR    
Version First Reported In: 3.14 SVN   
Target Milestone: ---   
Platform: Other   
OS: Linux   
Latest Commit: Version Fixed/Implemented In:
Sentry Crash Report:
Attachments: Fix register allocation for VRs vs FPRs

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.