fix compile=all and codegen bugs #22697
Conversation
vtjnash
added
backport pending 0.6
kind:bugfix
| @@ -480,7 +480,7 @@ static Value *runtime_apply_type(jl_codectx_t &ctx, jl_value_t *ty, jl_unionall_ | |||
| args[1] = literal_pointer_val(ctx, (jl_value_t*)ctx.linfo->def.method->sig); | |||
| args[2] = ctx.builder.CreateInBoundsGEP( | |||
| T_prjlvalue, | |||
| emit_bitcast(ctx, decay_derived(ctx.spvals_ptr), T_pprjlvalue), | |||
| ctx.spvals_ptr, | |||
There was a problem hiding this comment.
Are these guaranteed to be globally rooted?
There was a problem hiding this comment.
They're always callee rooted
There was a problem hiding this comment.
Is it OK to just assert that the argument type is T_pprjlvalue?
There was a problem hiding this comment.
I think you still need to decay_derived at the very least.
There was a problem hiding this comment.
It can just be considered an opaque (callee-rooted) pointer Type::getPointerTo(Void, 0). Unless the rooting pass cares that I'm loading a T_prjlvalue from it?
There was a problem hiding this comment.
Ok, as long as ctx.spvals_ptr is in addrspace(0) it's probably fine. The pass will just not root any values you load from it.
There was a problem hiding this comment.
That should be fine (preferred, actually). I assume it'll still wb them if they escape?
|
Is there a test we can meaningfully run as part of CI? |
|
Clearly building the system image and gadfly will kill CI. |
|
Will this fix #22569 or it's only a part of the fix? If it is the full fix, I can try this branch. |
|
Yes, I have built Gadfly with this |
|
Would be nice to get this merged to enable compile testing of more codes. |
| @@ -994,7 +994,6 @@ export | |||
| # loading source files | |||
| __precompile__, | |||
| evalfile, | |||
| include, | |||
There was a problem hiding this comment.
This wasn't supposed to be exported – it causes binding overwrite warnings when you try to extend the system image with compile all.
There was a problem hiding this comment.
could have mentioned that in the commit message or PR description, I suspect that's totally unclear to anyone other than you
fixes various issues that have bitrotted a bit since compile=all testing is not being done on CI.