Message ID | 1432031208-20020-1-git-send-email-aurelien@aurel32.net |
---|---|
State | New |
Headers | show |
On 05/19/2015 03:26 AM, Aurelien Jarno wrote: > @@ -1522,6 +1522,9 @@ static void tcg_liveness_analysis(TCGContext *s) > if (dead_temps[arg]) { > dead_args |= (1 << i); > } > + } > + for (i = nb_oargs; i < nb_oargs + nb_iargs; i++) { > + arg = args[i]; > dead_temps[arg] = 0; > } > s->op_dead_args[oi] = dead_args; How about another line of commentary for each loop? Something like /* Record arguments that die in this opcode. */ for the first and /* Input arguments are live for preceeding opcodes. */ for the second. As for the same loop for calls, you're right that it may well cause us to do a tiny bit of redundant work, but nothing else bad will happen. We'll enter temp_dead more times than necessary. I'm always skeptical about knowingly giving a compiler bad information though. You tend to not know how data is going to be used in future, and *then* get bad results. r~
On 2015-05-19 07:46, Richard Henderson wrote: > On 05/19/2015 03:26 AM, Aurelien Jarno wrote: > > @@ -1522,6 +1522,9 @@ static void tcg_liveness_analysis(TCGContext *s) > > if (dead_temps[arg]) { > > dead_args |= (1 << i); > > } > > + } > > + for (i = nb_oargs; i < nb_oargs + nb_iargs; i++) { > > + arg = args[i]; > > dead_temps[arg] = 0; > > } > > s->op_dead_args[oi] = dead_args; > > How about another line of commentary for each loop? > > Something like > > /* Record arguments that die in this opcode. */ > > for the first and > > /* Input arguments are live for preceeding opcodes. */ > > for the second. Good point. > As for the same loop for calls, you're right that it may well cause us to do a > tiny bit of redundant work, but nothing else bad will happen. We'll enter > temp_dead more times than necessary. I'm always skeptical about knowingly > giving a compiler bad information though. You tend to not know how data is > going to be used in future, and *then* get bad results. You are correct. Anyway I don't think it'll make a big difference in performance. I'll send a new version of the patch.
diff --git a/tcg/tcg.c b/tcg/tcg.c index f1558b7..58ca693 100644 --- a/tcg/tcg.c +++ b/tcg/tcg.c @@ -1522,6 +1522,9 @@ static void tcg_liveness_analysis(TCGContext *s) if (dead_temps[arg]) { dead_args |= (1 << i); } + } + for (i = nb_oargs; i < nb_oargs + nb_iargs; i++) { + arg = args[i]; dead_temps[arg] = 0; } s->op_dead_args[oi] = dead_args;
When the same temp is used twice or more as an input argument to a TCG instruction, the dead computation code doesn't recognize the second use as a dead temp. This is because the temp is marked as live in the same loop where dead inputs are checked. The fix is to split the loop in two parts. This avoid emitting a move and using a register for the movcond instruction when used as "move if true" on x86-64. This might bring more improvements on RISC TCG targets which don't have outputs aliased to inputs. Cc: Richard Henderson <rth@twiddle.net> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> --- tcg/tcg.c | 3 +++ 1 file changed, 3 insertions(+) NB: The dead computation loop for call has the same problem, but it will not improve the generated code as anyway the value has to be copied to a register or memory. I am therefore not sure it is worth fixing it as it might bring some performance penalty.