Comments (16)
SeanTAllen
commented on May 30, 2024
I am able to reproduce this on a 64 bit Raspberry PI with 976cc036 as the commit, so this goes back a ways.
from ponyc.
SeanTAllen
commented on May 30, 2024
I put some debugging in before the statements and then the code worked. In particular I changed to:
fun univals(): USize =>
let left_univals = _left.univals()
let right_univals = _right.univals()
if _left.get_value() == _right.get_value() then
@printf("first as expected\n".cstring())
end
if _left.get_value() == this.get_value() then
@printf("second as expected\n".cstring())
end
if (_left.get_value() == _right.get_value())
and (_left.get_value() == this.get_value()) then
1 + left_univals + right_univals
else
left_univals + right_univals
endfrom ponyc.
SeanTAllen
commented on May 30, 2024
the following also fails so it isn't related to the conjunction.
fun univals(): USize =>
let left_univals = _left.univals()
let right_univals = _right.univals()
if _left.get_value() == _right.get_value() then
if _left.get_value() == this.get_value() then
1 + left_univals + right_univals
else
left_univals + right_univals
end
else
left_univals + right_univals
endVery odd that printf's fix the issue.
from ponyc.
SeanTAllen
commented on May 30, 2024
I attempted a more minimal reproduction without the use of pony test code and it works as expected. So there's something very weird going on here.
use "pony_test"
interface Tree
fun get_value(): USize
fun univals(): USize
fun print_values(env: Env)
class Leaf is Tree
let _value: USize
new create(value: USize) =>
_value = value
fun print_values(env: Env) =>
None
fun get_value(): USize => _value
fun univals(): USize => 1
class Node
let _value: USize
let _left: Tree
let _right: Tree
new create(value: USize, left: Tree, right: Tree) =>
_value = value
_left = left
_right = right
fun get_value(): USize => _value
fun univals(): USize =>
let left_univals = _left.univals()
let right_univals = _right.univals()
if (_left.get_value() == _right.get_value())
and (_left.get_value() == this.get_value()) then
1 + left_univals + right_univals
else
left_univals + right_univals
end
fun print_values(env: Env) =>
env.out.print(this.univals().string())
actor Main
new create(env: Env) =>
let x = Node(0, Leaf(0), Leaf(0)).univals()
env.out.print(x.string())from ponyc.
SeanTAllen
commented on May 30, 2024
Changing the test from being private to the package _TestUnivals to public TestUnivals also fixes the issue.
from ponyc.
SeanTAllen
commented on May 30, 2024
The bug only happens in release mode (with original code), not with --debug so its related to an optimization as well.
from ponyc.
SeanTAllen
commented on May 30, 2024
Turning off inlining fixes the issue, but that is often the case here.
from ponyc.
SeanTAllen
commented on May 30, 2024
This appears to also be triggered by a combination of inlining on as an optimization PLUS the heap to stack optimization.
That suggests to me that there might be something a little wonky in our IR we are generating (as for example, making the test class public works).
from ponyc.
SeanTAllen
commented on May 30, 2024
@jemc and I were looking at the IR for the working (TestUnivals) vs the not working (_TestUnivals) and at the point of LLVM IR the not working "2" is already in the IR, so we need to look for the bug pre the end of the IR pass.
from ponyc.
SeanTAllen
commented on May 30, 2024
I've found the 3 optimization steps that interact poorly and lead to this bug.
Moving the HeapToStack pass that is our custom pass from Peephole to Scalar Optimizer Late moves it after a dead store elimination that it interacts poorly with (and leads to our bug).
I have started a "not on GitHub" conversation with @jemc going that we started today when debugging together. If Joe thinks that changing the phase for when the HeapToStack runs is a good fix, I'll open a PR for it.
from ponyc.
SeanTAllen
commented on May 30, 2024
It should be noted that the playground always runs code as --debug and that is why it works there. There's no optimizations being applied and this is a bug related to the order of optimizations.
from ponyc.
SeanTAllen
commented on May 30, 2024
@jemc and I did some debugging on this (a couple days ago) and ended up at this point when we left off...
the "annotated by us IR" after the heap to stack pass seems fine:
define private fastcc ptr @_TestUnivals_ref_apply_oo(ptr nocapture readnone %this, ptr nocapture readonly dereferenceable(24) %h) unnamed_addr !dbg !7533 !pony.abi !4 {
entry:
%_leaf_right = alloca i8, i64 32, align 8
%_leaf_left = alloca i8, i64 32, align 8
store ptr @Leaf_Desc, ptr %_leaf_left, align 8
%_leaf_left_value = getelementptr inbounds %Leaf, ptr %_leaf_left, i64 0, i32 1, !dbg !7539
store i64 0, ptr %_leaf_left_value, align 8, !dbg !7539
store ptr @Leaf_Desc, ptr %_leaf_right, align 8
%_leaf_right_value = getelementptr inbounds %Leaf, ptr %_leaf_right, i64 0, i32 1, !dbg !7542
store i64 0, ptr %_leaf_right_value, align 8, !dbg !7542
%_leaf_left_univals = tail call fastcc i64 @Leaf_ref_univals_Z(ptr nonnull %_leaf_left), !dbg !7547
%_leaf_left_get_value = tail call fastcc i64 @Leaf_ref_get_value_Z(ptr nonnull %_leaf_left), !dbg !7548
%_leaf_right_get_value = tail call fastcc i64 @Leaf_ref_get_value_Z(ptr nonnull %_leaf_right), !dbg !7549
%8 = icmp eq i64 %_leaf_left_get_value, %_leaf_right_get_value
br i1 %8, label %sc_right.i, label %Node_ref_univals_Z.exit
sc_right.i: ; preds = %entry
%9 = icmp eq i64 %_leaf_left_get_value, 0
%10 = zext i1 %9 to i64
%spec.select.i = add i64 %_leaf_left_univals, %10
br label %Node_ref_univals_Z.exit
Node_ref_univals_Z.exit: ; preds = %entry, %sc_right.i
%.pn.i = phi i64 [ %_leaf_left_univals, %entry ], [ %spec.select.i, %sc_right.i ]
%_leaf_right_univals = tail call fastcc i64 @Leaf_ref_univals_Z(ptr nonnull %_leaf_right), !dbg !7550
%12 = add i64 %.pn.i, %_leaf_right_univals
%13 = tail call fastcc i1 @pony_test_TestHelper_val__check_eq_USize_val_oZZoob(ptr nonnull %h, ptr nonnull @19, i64 3, i64 %12, ptr nonnull @39, ptr nonnull @"$1$0_Inst"), !dbg !7555
call void @llvm.dbg.value(metadata ptr @None_Inst, metadata !5286, metadata !DIExpression()), !dbg !7556
ret ptr @None_Inst, !dbg !7558
}The IR after the dead store elimination is done is most certainly not ok...
define private fastcc ptr @_TestUnivals_ref_apply_oo(ptr nocapture readnone %this, ptr nocapture readonly dereferenceable(24) %h) unnamed_addr !dbg !7533 !pony.abi !4 {
entry:
%0 = alloca i8, i64 32, align 8
%1 = alloca i8, i64 32, align 8
%3 = tail call fastcc i64 @Leaf_ref_univals_Z(ptr nonnull %1), !dbg !7543
%4 = tail call fastcc i64 @Leaf_ref_get_value_Z(ptr nonnull %1), !dbg !7544
%5 = tail call fastcc i64 @Leaf_ref_get_value_Z(ptr nonnull %0), !dbg !7545
%6 = icmp eq i64 %4, %5
br i1 %6, label %sc_right.i, label %Node_ref_univals_Z.exit
sc_right.i: ; preds = %entry
%7 = icmp eq i64 %4, 0
%8 = zext i1 %7 to i64
%spec.select.i = add i64 %3, %8
br label %Node_ref_univals_Z.exit
Node_ref_univals_Z.exit: ; preds = %entry, %sc_right.i
%.pn.i = phi i64 [ %3, %entry ], [ %spec.select.i, %sc_right.i ]
%9 = tail call fastcc i64 @Leaf_ref_univals_Z(ptr nonnull %0), !dbg !7546
call void @llvm.dbg.value(metadata i64 %9, metadata !5822, metadata !DIExpression()), !dbg !7541
%10 = add i64 %.pn.i, %9
call void @llvm.dbg.value(metadata ptr %h, metadata !4679, metadata !DIExpression()), !dbg !7547
call void @llvm.dbg.value(metadata i64 3, metadata !4681, metadata !DIExpression()), !dbg !7547
call void @llvm.dbg.value(metadata i64 %10, metadata !4682, metadata !DIExpression()), !dbg !7547
call void @llvm.dbg.value(metadata ptr @39, metadata !4683, metadata !DIExpression()), !dbg !7547
call void @llvm.dbg.value(metadata ptr @"$1$0_Inst", metadata !4684, metadata !DIExpression()), !dbg !7547
%11 = tail call fastcc i1 @pony_test_TestHelper_val__check_eq_USize_val_oZZoob(ptr nonnull %h, ptr nonnull @19, i64 3, i64 %10, ptr nonnull @39, ptr nonnull @"$1$0_Inst"), !dbg !7549
ret ptr @None_Inst, !dbg !7550
}from ponyc.
SeanTAllen
commented on May 30, 2024
I looked into this more today and narrowed down the optimization in dead store elimination that leads to our issue and I'm a little gobsmacked. This needs more debugging, but the optimization is eliminateDeadWritesAtEndOfFunction.
from ponyc.
SeanTAllen
commented on May 30, 2024
I was looking into the public works, private doesn't a bit more and it isn't related to public v private BUT does appear to have some sort of correlation to the name, but there's no obvious pattern that I can figure out except that it might be related to some kind of sorting/ordering.
from ponyc.
SeanTAllen
commented on May 30, 2024
So the problem is caused by our inlining of the setup of fields for the objects. This comes from getopt.cc
for(auto new_call: new_calls)
{
// Force constructor inlining to see if fields can be stack-allocated.
InlineFunctionInfo ifi{};
auto new_call_base = dyn_cast<CallBase>(new_call);
if (new_call_base)
{
InlineFunction(*new_call_base, ifi);
}
}And from looking at our "good IR" and looking at what the eliminateDeadWritesAtEndOfFunction pass does. It makes sense that it gets eliminated. There's nothing about our inlined field setup that makes it appear anywhere in the function that they are used (but they are).
@jemc let's go over this together to see if you agree with my analysis.
I think what we have is...
- field setup is inlined as a store in function.
- field is still read via another function
- eliminate dead writes at end of function doesn't look into other functions to find reads of a store.
from ponyc.
SeanTAllen
commented on May 30, 2024
We have a fix coming for this. @jemc and I tracked down the bug during the sync call.
from ponyc.
Related Issues (20)
- Non-sendable data seen as sendable
- Release 0.57.1 HOT 2
- Generic parameter when copying a trait method into an object that has a generic parameter of the same name HOT 2
- return usage in disposing block's is flawed and we need to revist. HOT 5
- Slow compare to elixir in `ring benchmark` HOT 2
- Clean up MacOS platform version setting HOT 1
- Release 0.58.0 HOT 1
- Release 0.58.1
- Invalid LLVM generated HOT 1
- Segmentation fault when ponyc compiles this code HOT 2
- Compile fails verification "GEP base pointer is not a vector or a vector of pointers" HOT 13
- LLVM mangling error with `powi` HOT 4
- Release 0.58.2
- Constrained Types RFC
- Segfault from bad cap in code
- Release 0.58.3
- Release 0.58.4 HOT 1
- Segfault when accessing a union in a tuple via `Any` HOT 4
- Improve error message for failure of generic subtyping of concrete types HOT 8
- Release 0.58.5
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from ponyc.