Six months later...
On 8/21/19 12:41 PM, Matteo Bruni wrote:
On Tue, Aug 13, 2019 at 10:55 PM Zebediah Figura zfigura@codeweavers.com wrote:
Signed-off-by: Zebediah Figura zfigura@codeweavers.com
v2: don't drop all of the struct initializer elements on the floor
dlls/d3dcompiler_43/d3dcompiler_private.h | 6 + dlls/d3dcompiler_43/hlsl.y | 269 ++++++++++++---------- dlls/d3dcompiler_43/utils.c | 45 +--- 3 files changed, 155 insertions(+), 165 deletions(-)
@@ -315,7 +313,7 @@ enum loop_type };
static struct list *create_loop(enum loop_type type, struct list *init, struct list *cond,
struct hlsl_ir_node *iter, struct list *body, struct source_location *loc)
struct list *iter, struct list *body, struct source_location *loc){ struct list *list = NULL; struct hlsl_ir_loop *loop = NULL; @@ -345,15 +343,15 @@ static struct list *create_loop(enum loop_type type, struct list *init, struct l goto oom;
if (type != LOOP_DO_WHILE)
list_add_tail(loop->body, &cond_jump->node.entry);
list_move_tail(loop->body, cond);Should this use cond_jump? Right now cond_jump seems unused and cond is freed at the end of the function. FWIW cond_jump is really a misnomer, it's a struct hlsl_ir_if *.
No, because we want to append the whole condition expression (i.e. the list containing all of the condition nodes as well as the branch/jump.) "cond_jump" is already tacked onto the end of "cond" by loop_condition(). I will agree these functions are not particularly clear; we could probably do better by returning BOOL from loop_condition() and renaming it to something like make_conditional_break(). I'll make that change in v3.
list_move_tail(loop->body, body); if (iter)
list_add_tail(loop->body, &iter->entry);
list_move_tail(loop->body, iter);if (type == LOOP_DO_WHILE)
list_add_tail(loop->body, &cond_jump->node.entry);
list_move_tail(loop->body, cond);Same.
diff --git a/dlls/d3dcompiler_43/utils.c b/dlls/d3dcompiler_43/utils.c index 1dea5ba5bf8..8f069b17ec7 100644 --- a/dlls/d3dcompiler_43/utils.c +++ b/dlls/d3dcompiler_43/utils.c @@ -1277,10 +1277,13 @@ static struct hlsl_type *expr_common_type(struct hlsl_type *t1, struct hlsl_type static struct hlsl_ir_node *implicit_conversion(struct hlsl_ir_node *node, struct hlsl_type *type, struct source_location *loc) {
- struct hlsl_ir_expr *cast; if (compare_hlsl_types(node->data_type, type)) return node; TRACE("Implicit conversion of expression to %s\n", debug_hlsl_type(type));
- return &new_cast(node, type, loc)->node;
- if ((cast = new_cast(node, type, loc)))
list_add_after(&node->entry, &cast->node.entry);- return &cast->node;
}
Nitpick, can you leave a blank line between the declarations and the code?
Yep, done.
Aside from those points, I like the patch and the IR flattening in particular. Nice job!