hlsl: Function inlining.
This is the last piece that's needed for function calls to basically work.
Merge request reports
Activity
551 * the CF instruction, shove it into an if block, and then lower that if 552 * block. 553 * 554 * (We could return a "did we make progress" like transform_ir() and run 555 * this pass multiple times, but we already know the only block that still 556 * needs addressing, so there's not much point.) 557 * 558 * If we're inside of a loop CF block, we again do things differently. We 559 * already turned any returns into breaks. If the block we just processed 560 * was conditional, then "break" did our work for us. If it was a loop, 561 * we need to propagate that break to the outer loop. 562 */ 563 564 LIST_FOR_EACH_ENTRY_SAFE(instr, next, &block->instrs, struct hlsl_ir_node, entry) 565 { 566 if (instr->type == HLSL_IR_CALL) 564 LIST_FOR_EACH_ENTRY_SAFE(instr, next, &block->instrs, struct hlsl_ir_node, entry) 565 { 566 if (instr->type == HLSL_IR_CALL) 567 { 568 struct hlsl_ir_call *call = hlsl_ir_call(instr); 569 570 lower_return(ctx, call->decl, &call->decl->body, false); 571 } 572 else if (instr->type == HLSL_IR_IF) 573 { 574 struct hlsl_ir_if *iff = hlsl_ir_if(instr); 575 576 lower_return(ctx, func, &iff->then_instrs, in_loop); 577 lower_return(ctx, func, &iff->else_instrs, in_loop); 578 579 if (func->early_return_var) I think I would find it more legible if we stipulated that
early_return_var
always exists, so you can skip a few branches in this function, which is already relatively complicated. This would leave in some more dead code, but hopefully there will eventually be a dead if elimination pass that gets rid of it.That said, nice function! Before reading that I thought it would have been much more of a pain.
- Resolved by Giovanni Mascellani
- Resolved by Giovanni Mascellani
- Resolved by Giovanni Mascellani
12 12 RWTexture2D<float4> u; 13 13 14 void func(inout float4 f) 14 void func(out float4 f) 15 15 { 16 f.xz += 0.1; 16 f.x = 0.1; 17 f.y = 0.2; 18 f.z = 0.3; 19 f.w = 0.4; 17 20 } 18 21 19 22 [numthreads(1, 1, 1)] 20 23 void main() 21 24 { 22 25 func(u[uint2(0, 0)].yzwx); The problem isn't that it doesn't compile, it's that it requires an optional d3d11/d3d12 feature to run. (It probably works on 47 and not on earlier versions because earlier versions predate that d3d11 version.) In practice it also fails in the Vulkan backend for basically that reason. We could add support in the shader runner [require] block to check for that feature, but it seemed easier to just change the test; it should still check the same things I was trying to test in the first place (i.e. swizzles work, in/out parameters work...)
2685 2690 return VKD3D_ERROR_NOT_IMPLEMENTED; 2686 2691 } 2687 2692 2688 if (!(entry_func = hlsl_get_func_decl(&ctx, entry_point))) 2693 if ((func = hlsl_get_function(&ctx, entry_point))) 2694 { 2695 RB_FOR_EACH_ENTRY(decl, &func->overloads, struct hlsl_ir_function_decl, entry) 2696 { 2697 if (!decl->has_body) 2698 continue; 2699 entry_func = decl; 2700 break; 2701 } - Comment on lines +2695 to +2701
Since, in case of multiple overloads with definitions, we are still selecting the one that comes first in the order given by
compare_function_decl_rb()
-- which depends on the parameter types -- but in !65 (merged) it was mentioned that in native it depends on the .dll version and the order in which the declarations appear, I would suggest displaying aFIXME
:RB_FOR_EACH_ENTRY(decl, &func->overloads, struct hlsl_ir_function_decl, entry) { if (!decl->has_body) continue; if (entry_func) { FIXME("Multiple valid entry point definitions.\n"); break; } entry_func = decl; }
429 429 * Not to be confused with the function parameters! */ 430 430 unsigned int attr_count; 431 431 const struct hlsl_attribute *const *attrs; 432 struct hlsl_ir_var *early_return_var; 2401 2401 break; 2402 2402 2403 2403 default: 2404 FIXME("Unhandled instruction type %s.\n", hlsl_node_type_to_string(instr->type)); 2404 hlsl_fixme(ctx, &instr->loc, "Instruction type %s.", hlsl_node_type_to_string(instr->type)); 2927 3126 transform_ir(ctx, find_recursive_calls, body, &recursive_call_ctx); 2928 3127 vkd3d_free(recursive_call_ctx.backtrace); 2929 3128 3129 /* Avoid going into an infinite loop when processing them. */ changed this line in version 2 of the diff
541 * statement in this block. We have to stop and do this in a separate 542 * loop, because instructions must be deleted in reverse order (due to 543 * def-use chains.) 544 * 545 * If we're inside of a loop CF block, we can instead just turn the 546 * return into a break, which offers the right semantics—except that it 547 * won't break out of nested loops. 548 * 549 * - A CF block which might contain a return statement. After calling 550 * lower_return() on the CF block body, we stop, pull out everything after 551 * the CF instruction, shove it into an if block, and then lower that if 552 * block. 553 * 554 * (We could return a "did we make progress" like transform_ir() and run 555 * this pass multiple times, but we already know the only block that still 556 * needs addressing, so there's not much point.) - Comment on lines +554 to +556
The previous part of the comment is great at explaining what this does!
This paragraph got me a little confused at first though, because I didn't made the association with the
progress
flag immediately. Perhaps a rephrasing would help:We could return a "did we make progress" boolean like in transform_ir() and run this pass multiple times, but we already know the only block that remains to be addressed, so there's not much point.
Also, I have the feeling that this approach wouldn't be easy either, because we would have to check if we already inserted the
early_return_var
conditionals during the last iteration. changed this line in version 2 of the diff
The previous part of the comment is great at explaining what this does!
Thanks, I'm glad to hear that. Explaining the code was probably my biggest concern when writing this; it's not a simple algorithm.
This paragraph got me a little confused at first though, because I didn't made the association with the
progress
flag immediately. Perhaps a rephrasing would help:We could return a "did we make progress" boolean like in transform_ir() and run this pass multiple times, but we already know the only block that remains to be addressed, so there's not much point.
Yeah, the original is at least missing a word; I've adjusted it a little.
added 13 commits
-
71a09412...902ddee5 - 5 commits from branch
wine:master
- 9e2128dd - tests: Avoid performing a multi-component UAV load in uav-out-param.shader_test.
- cd2ba9bf - vkd3d-shader/hlsl: Skip functions that don't have a body when looking for the entry point.
- 9aa0cee4 - vkd3d-shader/hlsl: Emit a hlsl_fixme() if multiple valid entry point definitions are given.
- d8e27964 - vkd3d-shader/hlsl: Lower return statements.
- c031993d - vkd3d-shader/hlsl: Emit a hlsl_fixme() for unhandled instruction types when writing bytecode.
- 5336fcf0 - vkd3d-shader/hlsl: Inline function calls.
- 6f8044dd - vkd3d-shader/hlsl: Handle early return after a CF block only if there was...
- d2fc39a2 - vkd3d-shader/hlsl: Allow the final expression in a for loop initializer to be omitted.
Toggle commit list-
71a09412...902ddee5 - 5 commits from branch
1280 hlsl_src_from_node(&dst->path[i], map_instr(map, src->path[i].node)); 1281 1282 return true; 1283 } 1284 1285 static void clone_src(struct clone_instr_map *map, struct hlsl_src *dst, const struct hlsl_src *src) 1286 { 1287 hlsl_src_from_node(dst, map_instr(map, src->node)); 1288 } 1289 1290 static struct hlsl_ir_node *clone_call(struct hlsl_ctx *ctx, struct hlsl_ir_call *src) 1291 { 1292 struct hlsl_ir_node *dst; 1293 1294 return hlsl_new_call(ctx, src->decl, &src->node.loc); 1295 } added 5 commits
- a45414bc - vkd3d-shader/hlsl: Lower return statements.
- 49fa7373 - vkd3d-shader/hlsl: Emit a hlsl_fixme() for unhandled instruction types when writing bytecode.
- 35152f49 - vkd3d-shader/hlsl: Inline function calls.
- 60944e5e - vkd3d-shader/hlsl: Handle early return after a CF block only if there was...
- 375ff9f3 - vkd3d-shader/hlsl: Allow the final expression in a for loop initializer to be omitted.
Toggle commit list