| Summary: | NV50 Gallium codegen RA incorrectly tracks interference for 64-bit / 128-bit registers | ||
|---|---|---|---|
| Product: | Mesa | Reporter: | Jay Cornwall <jay> |
| Component: | Other | Assignee: | mesa-dev |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | medium | ||
| Version: | git | ||
| Hardware: | Other | ||
| OS: | All | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
| Attachments: |
Test case to reproduce register interference
Fix NV50 RA interference register tracking. |
||
Created attachment 67549 [details] [review] Fix NV50 RA interference register tracking. Patch applied, thank you ! |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.
Created attachment 67548 [details] Test case to reproduce register interference Problem: The attached test case reproduces a problem in Gallium NV50 (+NVC0) codegen with register interference tracking for a 64-bit register. The compiler maintains a live register across the execution of a built-in function call which has declared its intention to clobber it. Explanation: nv50_ir_ra.cpp:1238 in GCRA::checkInterference uses RegisterSet::occupy to track registers used by interfering nodes. It is possible for the following situation to arise: * r0 is set in the RegisterSet * RegisterSet::occupy is called to set r0d as well RegisterSet::occupy will fail to mark r0d as set, by noting the overlap with r0 during the call to BitSet::testRange. This is correct behavior for other users of RegisterSet::occupy. However, failure is neither checked nor desirable for GCRA::checkInterference. It doesn't matter that two other nodes use overlapping registers - just that the RegisterSet tracks the union of the two as occupied. Solution: The attached patch proposes a fix for this. It adds a fourth parameter to RegisterSet::occupy to skip the BitSet::testRange check as the callee desires. This may not be an optimal solution and I have not confirmed whether similar problems could arise for other users of RegisterSet::occupy. This patch only affects the behavior of the single, known problematic call site.