<html>
<head>
<base href="https://bugs.llvm.org/">
</head>
<body><table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>Bug ID</th>
<td><a class="bz_bug_link
bz_status_NEW "
title="NEW - [Statepoint-VReg] Cornercase bug with multiple uses of a single value"
href="https://bugs.llvm.org/show_bug.cgi?id=46917">46917</a>
</td>
</tr>
<tr>
<th>Summary</th>
<td>[Statepoint-VReg] Cornercase bug with multiple uses of a single value
</td>
</tr>
<tr>
<th>Product</th>
<td>libraries
</td>
</tr>
<tr>
<th>Version</th>
<td>trunk
</td>
</tr>
<tr>
<th>Hardware</th>
<td>PC
</td>
</tr>
<tr>
<th>OS</th>
<td>Linux
</td>
</tr>
<tr>
<th>Status</th>
<td>NEW
</td>
</tr>
<tr>
<th>Severity</th>
<td>enhancement
</td>
</tr>
<tr>
<th>Priority</th>
<td>P
</td>
</tr>
<tr>
<th>Component</th>
<td>Common Code Generator Code
</td>
</tr>
<tr>
<th>Assignee</th>
<td>unassignedbugs@nondot.org
</td>
</tr>
<tr>
<th>Reporter</th>
<td>listmail@philipreames.com
</td>
</tr>
<tr>
<th>CC</th>
<td>llvm-bugs@lists.llvm.org
</td>
</tr></table>
<p>
<div>
<pre>There is a hard to handle cornercase in our experimental vreg statepoint
lowering. I had briefly described the problem in
<a href="https://reviews.llvm.org/D81648?vs=on&id=277132#2146051">https://reviews.llvm.org/D81648?vs=on&id=277132#2146051</a> as such:
"Second, there appears to be a semantic problem around the handling of base vs
derived slots unless we *always* spill the base. We can't tie both uses to a
single def. This may warrant some offline discussion."
(I decided to land that patch with the issue unresolved to unblock progress
since review discussion was fragmented and hard to follow.)
Let me expand a bit on the issue. Say we have an example like the following:
target datalayout = "e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-pc-linux-gnu"
declare void @foo()
declare void @consume(i8 addrspace(1)*)
define void @test(i8 addrspace(1)* %a) gc "statepoint-example" {
entry:
%a_gep = getelementptr i8, i8 addrspace(1)* %a, i64 8
%safepoint_token = tail call token (i64, i32, void ()*, i32, i32, ...)
@llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* @foo, i32
0, i32 0, i32 0, i32 0) ["gc-live" (i8 addrspace(1)* %a, i8 addrspace(1)*
%a_gep)]
%a1 = call i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token
%safepoint_token, i32 0, i32 1)
call void @consume(i8 addrspace(1)* %a1)
ret void
}
declare token @llvm.experimental.gc.statepoint.p0f_i1f(i64, i32, i1 ()*, i32,
i32, ...)
declare token @llvm.experimental.gc.statepoint.p0f_isVoidf(i64, i32, void ()*,
i32, i32, ...)
declare i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token, i32, i32)
In this example, we generate a statepoint MI node which looks like this:
%6:gr64 = STATEPOINT 0, 0, 0, @foo, 2, 0, 2, 0, 2, 0, 1, 8, %stack.0, 0, killed
%7(tied-def 0), csr_64, implicit-def $rsp, implicit-def $ssp :: (volatile load
store 8 on %stack.0)
(Where %7 is the GEP, and %stack.0 is the spill for the base)
Imagine we had instead generated:
%6:gr64 = STATEPOINT 0, 0, 0, @foo, 2, 0, 2, 0, 2, 0, 1, 0, %7, 0, killed
%7(tied-def 0), csr_64, implicit-def $rsp, implicit-def $ssp :: (volatile load
store 8 on %stack.0)
(That is, replaced stack with another usage of %7.)
This would be incorrect. Why? Because the second use of %7 can not be tied to
the %6 def. As a result, the fact that the GC may need to update the base -
remember objects may move many times during a call - is lost. The register
allocator is free to assign different locations to the the two uses of %0, and
then *assume one of them is read only*. That would be a miscompile.
To say all that different, we tie *operands*, not registers.
Unfortunately, the very slightly tweaked example produces exactly this effect:
define void @test(i8 addrspace(1)* %a) gc "statepoint-example" {
entry:
%a_gep = getelementptr i8, i8 addrspace(1)* %a, i64 8
%safepoint_token = tail call token (i64, i32, void ()*, i32, i32, ...)
@llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* @foo, i32
0, i32 0, i32 0, i32 0) ["gc-live" (i8 addrspace(1)* %a, i8 addrspace(1)*
%a_gep)]
%a1 = call i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token
%safepoint_token, i32 0, i32 0)
call void @consume(i8 addrspace(1)* %a1)
ret void
}
This produces:
%1:gr64 = STATEPOINT 0, 0, 0, @foo, 2, 0, 2, 0, 2, 0, %0, %0(tied-def 0),
csr_64, implicit-def $rsp, implicit-def $ssp
Which is WRONG.
One possible correct alternative would be:
%1:gr64, %2:gr64 = STATEPOINT 0, 0, 0, @foo, 2, 0, 2, 0, 2, 0, %0(tied-def 0),
%0(tied-def 1), csr_64, implicit-def $rsp, implicit-def $ssp
Note that while I've given the example in terms of a single base/derived pair,
you can also construct the same problematic pattern by relocating a single
object twice (or otherwise having multiple gc operand uses of the same value).
The two major families of fixes I see are:
1) Produce a separate tied def for each use copy.
2) Produce at most one use of each value - thus, eagerly spilling further uses.
The second seems like a hard invariant to preserve during optimization, so I'd
tend towards the first.
The first does result in slightly poor codegen since we can end up spilling the
same value twice. I don't have a simple fix for that. We could potentially
change the statepoint MI format to remove the dual use, but that's a bit
involved.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are on the CC list for the bug.</li>
</ul>
</body>
</html>