Better C++ compatibility and fixes to conditional branch/switch handling #10
Conversation
src/opcode_execute.c
Outdated
| state->function_stack_cfg[state->function_stack_current] = id; | ||
|
|
||
| // Make sure we do not overwrite parent block id if we encounter an explicit label after a jump | ||
| if (state->function_stack_cfg[state->function_stack_current] != id) { |
There was a problem hiding this comment.
do we ever take this branch (except maybe for the entry point)? Couldn't we always handle the update of function_stack_cfg_parent, function_stack_cfg here instead of handling it in every jump instruction?
There was a problem hiding this comment.
Indeed, it seems this can be done. When you debug current code you will see that branch target 'source locations' are not actual OpLabel instructions. This is due to the setup phase (spvm_setup_OpLabel) that moves source_location of its result record into the next instruction.
After changing the corresponding line in spvm_setup_OpLabel
state->results[id].source_location = state->code_current;
to
state->results[id].source_location = state->code_current - 2;
the code in actual branch instructions can be removed.
Should I update the PR?
There was a problem hiding this comment.
The PR is valuable and LGTM as it is, no need to change it right away. IMO doing the processing in OpLabel (and changing source_location as you proposed) would simplify the code though as it removes duplication but we can also do this later on if you prefer. Thanks for looking into it!
There was a problem hiding this comment.
Thanks. I already pushed this change, it really keeps the code cleaner.
…ranch instructions becomes much simpler this way.
…of the input). Implemented OpCompositeInsert (Khronos compiler generates this is 'size optimization' mode). Also perform full 64-bit copy in OpCompositeConstruct, instead of 32-bit copy.
|
I added couple of additional fixes to my branch and implementation for |
|
@dfranx any chance of merging this? Fixes a major bug with OpPhi |
…nstruct, GLSL450_SmoothStep. Implemented unordered floating point comparison operators.
|
@dfranx And chance we can get some traction on this? |
No description provided.