Skip to content

inference: follow up #44515, correctly encode Effects.overlayed effect #44634

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 17, 2022
Merged

inference: follow up #44515, correctly encode Effects.overlayed effect #44634

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions base/boot.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -423,10 +423,10 @@ eval(Core, quote
function CodeInstance(
mi::MethodInstance, @nospecialize(rettype), @nospecialize(inferred_const),
@nospecialize(inferred), const_flags::Int32, min_world::UInt, max_world::UInt,
ipo_effects::UInt8, effects::UInt8, @nospecialize(argescapes#=::Union{Nothing,Vector{ArgEscapeInfo}}=#),
ipo_effects::UInt32, effects::UInt32, @nospecialize(argescapes#=::Union{Nothing,Vector{ArgEscapeInfo}}=#),
relocatability::UInt8)
return ccall(:jl_new_codeinst, Ref{CodeInstance},
(Any, Any, Any, Any, Int32, UInt, UInt, UInt8, UInt8, Any, UInt8),
(Any, Any, Any, Any, Int32, UInt, UInt, UInt32, UInt32, Any, UInt8),
mi, rettype, inferred_const, inferred, const_flags, min_world, max_world,
ipo_effects, effects, argescapes,
relocatability)
Expand Down
3 changes: 3 additions & 0 deletions base/compiler/compiler.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ include("operators.jl")
include("pointer.jl")
include("refvalue.jl")

# the same constructor as defined in float.jl, but with a different name to avoid redefinition
_Bool(x::Real) = x==0 ? false : x==1 ? true : throw(InexactError(:Bool, Bool, x))

# checked arithmetic
const checked_add = +
const checked_sub = -
Expand Down
28 changes: 15 additions & 13 deletions base/compiler/types.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,31 +78,33 @@ function Effects(e::Effects = EFFECTS_UNKNOWN;
end

is_total_or_error(effects::Effects) =
effects.consistent === ALWAYS_TRUE && effects.effect_free === ALWAYS_TRUE &&
effects.consistent === ALWAYS_TRUE &&
effects.effect_free === ALWAYS_TRUE &&
effects.terminates === ALWAYS_TRUE

is_total(effects::Effects) =
is_total_or_error(effects) && effects.nothrow === ALWAYS_TRUE
is_total_or_error(effects) &&
effects.nothrow === ALWAYS_TRUE

is_removable_if_unused(effects::Effects) =
effects.effect_free === ALWAYS_TRUE &&
effects.terminates === ALWAYS_TRUE &&
effects.nothrow === ALWAYS_TRUE

function encode_effects(e::Effects)
return (e.consistent.state << 1) |
(e.effect_free.state << 3) |
(e.nothrow.state << 5) |
(e.terminates.state << 7) |
(e.overlayed)
return (e.consistent.state << 0) |
(e.effect_free.state << 2) |
(e.nothrow.state << 4) |
(e.terminates.state << 6) |
(UInt32(e.overlayed) << 8)
end
function decode_effects(e::UInt8)
function decode_effects(e::UInt32)
return Effects(
TriState((e >> 1) & 0x03),
TriState((e >> 3) & 0x03),
TriState((e >> 5) & 0x03),
TriState((e >> 7) & 0x03),
e & 0x01 ≠ 0x00,
TriState((e >> 0) & 0x03),
TriState((e >> 2) & 0x03),
TriState((e >> 4) & 0x03),
TriState((e >> 6) & 0x03),
_Bool( (e >> 8) & 0x01),
false)
end

Expand Down
8 changes: 4 additions & 4 deletions src/dump.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -618,8 +618,8 @@ static void jl_serialize_code_instance(jl_serializer_state *s, jl_code_instance_

write_uint8(s->s, TAG_CODE_INSTANCE);
write_uint8(s->s, flags);
write_uint8(s->s, codeinst->ipo_purity_bits);
write_uint8(s->s, codeinst->purity_bits);
write_uint32(s->s, codeinst->ipo_purity_bits);
write_uint32(s->s, codeinst->purity_bits);
jl_serialize_value(s, (jl_value_t*)codeinst->def);
if (write_ret_type) {
jl_serialize_value(s, codeinst->inferred);
Expand Down Expand Up @@ -1830,8 +1830,8 @@ static jl_value_t *jl_deserialize_value_code_instance(jl_serializer_state *s, jl
int flags = read_uint8(s->s);
int validate = (flags >> 0) & 3;
int constret = (flags >> 2) & 1;
codeinst->ipo_purity_bits = read_uint8(s->s);
codeinst->purity_bits = read_uint8(s->s);
codeinst->ipo_purity_bits = read_uint32(s->s);
codeinst->purity_bits = read_uint32(s->s);
codeinst->def = (jl_method_instance_t*)jl_deserialize_value(s, (jl_value_t**)&codeinst->def);
jl_gc_wb(codeinst, codeinst->def);
codeinst->inferred = jl_deserialize_value(s, &codeinst->inferred);
Expand Down
4 changes: 2 additions & 2 deletions src/gf.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ JL_DLLEXPORT jl_code_instance_t* jl_new_codeinst(
jl_method_instance_t *mi, jl_value_t *rettype,
jl_value_t *inferred_const, jl_value_t *inferred,
int32_t const_flags, size_t min_world, size_t max_world,
uint8_t ipo_effects, uint8_t effects, jl_value_t *argescapes,
uint32_t ipo_effects, uint32_t effects, jl_value_t *argescapes,
uint8_t relocatability);
JL_DLLEXPORT void jl_mi_cache_insert(jl_method_instance_t *mi JL_ROOTING_ARGUMENT,
jl_code_instance_t *ci JL_ROOTED_ARGUMENT JL_MAYBE_UNROOTED);
Expand Down Expand Up @@ -390,7 +390,7 @@ JL_DLLEXPORT jl_code_instance_t *jl_new_codeinst(
jl_method_instance_t *mi, jl_value_t *rettype,
jl_value_t *inferred_const, jl_value_t *inferred,
int32_t const_flags, size_t min_world, size_t max_world,
uint8_t ipo_effects, uint8_t effects, jl_value_t *argescapes,
uint32_t ipo_effects, uint32_t effects, jl_value_t *argescapes,
uint8_t relocatability
/*, jl_array_t *edges, int absolute_max*/)
{
Expand Down
2 changes: 1 addition & 1 deletion src/jltypes.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2522,7 +2522,7 @@ void jl_init_types(void) JL_GC_DISABLED
jl_any_type,
//jl_any_type,
//jl_bool_type,
jl_uint8_type, jl_uint8_type,
jl_uint32_type, jl_uint32_type,
jl_any_type,
jl_bool_type,
jl_bool_type,
Expand Down
6 changes: 4 additions & 2 deletions src/julia.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -394,21 +394,23 @@ typedef struct _jl_code_instance_t {

// purity results
union {
uint8_t ipo_purity_bits;
uint32_t ipo_purity_bits;
struct {
uint8_t ipo_consistent:2;
uint8_t ipo_effect_free:2;
uint8_t ipo_nothrow:2;
uint8_t ipo_terminates:2;
uint8_t ipo_overlayed:1;
} ipo_purity_flags;
};
union {
uint8_t purity_bits;
uint32_t purity_bits;
struct {
uint8_t consistent:2;
uint8_t effect_free:2;
uint8_t nothrow:2;
uint8_t terminates:2;
uint8_t overlayed:1;
} purity_flags;
};
jl_value_t *argescapes; // escape information of call arguments
Expand Down
12 changes: 12 additions & 0 deletions src/serialize.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,18 @@ static uint16_t read_uint16(ios_t *s) JL_NOTSAFEPOINT
return x;
}

static void write_uint32(ios_t *s, uint32_t i) JL_NOTSAFEPOINT
{
ios_write(s, (char*)&i, 4);
}

static uint32_t read_uint32(ios_t *s) JL_NOTSAFEPOINT
{
uint32_t x = 0;
ios_read(s, (char*)&x, 4);
return x;
}

Comment on lines +112 to +123
Copy link
Member

@giordano giordano Mar 24, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These functions are duplicates of

julia/src/staticdata.c

Lines 371 to 381 in c503611

static void write_uint32(ios_t *s, uint32_t i) JL_NOTSAFEPOINT
{
ios_write(s, (char*)&i, 4);
}
static uint32_t read_uint32(ios_t *s) JL_NOTSAFEPOINT
{
uint32_t x = 0;
ios_read(s, (char*)&x, 4);
return x;
}
Also, these function introduced a new warning when compiling julia: this header file is included by ircode.c which doesn't use these functions. I'm not even sure why there are functions implementations in a header.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the best way in C to define utility functions in a single place and import them from several places? In this case, dump.c uses all write_xxx utilities defined in serialize.h while ircode.c only uses some of them and so #include serialize.h produced that warning (and the same thing would happen for staticdata.c if we remove the duplications).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the best way in C to define utility functions in a single place and import them from several places?

Move the implementations (not the prototypes!) to a .c file, and make the files which need those functions depend on its corresponding object file in src/Makefile

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't the reason it is done like this to make the function definition available in all translation units and thus be eligible for inlining? But these functions are not marked inline though... But, to me, for consistency, the function in staticdata.c should be removed and this header included there instead?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#44739

void *jl_lookup_ser_tag(jl_value_t *v);
void *jl_lookup_common_symbol(jl_value_t *v);
jl_value_t *jl_deser_tag(uint8_t tag);
Expand Down