On 16 August 2018 at 13:05, Roman Kapl <r...@sysgo.com> wrote:
> If an instruction is conditional (like CBZ) and it is executed conditionally
> (using the ITx instruction), a jump to undefined label is generated.
> CBZ in IT block is an unpredictable behavior, and honouring the condition code
> is allowed by the spec in this case (constrained unpredictable, ARMv8, section
> K1.1.7).
>
> Fix the 'skip on condtion' code to create a new label only if it does not
> already exist. Previously multiple labels were created, but only the last one
> of
> them was set.
>
> Signed-off-by: Roman Kapl <r...@sysgo.com>
> ---
> v1 -> v2
> Split arm_conditional_skip into arm_gen_condlabel and arm_skip_unless to
> a) cover all usages
> b) do not force callers to ^1 the condition
>
> Add note about CBZ in IT block to the commit message.
>
> target/arm/translate.c | 35 +++++++++++++++++++++--------------
> 1 file changed, 21 insertions(+), 14 deletions(-)
>
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index f845da7c63..e54e0ca2ba 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -8480,6 +8480,22 @@ static void gen_srs(DisasContext *s,
> s->base.is_jmp = DISAS_UPDATE;
> }
>
> +/* Generate a labe used for skipping this instruction */
"label"
> +static void arm_gen_condlabel(DisasContext *s)
> +{
> + if (!s->condjmp) {
> + s->condlabel = gen_new_label();
> + s->condjmp = 1;
> + }
> +}
> +
> +/* Skip this instruction if the ARM condition is false */
> +static void arm_skip_unless(DisasContext *s, uint32_t cond)
> +{
> + arm_gen_condlabel(s);
> + arm_gen_test_cc(cond, s->condlabel ^ 1);
This has applied the ^ 1 to the wrong argument. This causes
a compile error (at least if you're building with debug):
/home/petmay01/linaro/qemu-from-laptop/qemu/target/arm/translate.c: In
function ‘arm_skip_unless’:
/home/petmay01/linaro/qemu-from-laptop/qemu/target/arm/translate.c:8496:40:
error: invalid operands to binary ^ (have ‘TCGLabel * {aka struct
TCGLabel *}’ and ‘int’)
arm_gen_test_cc(cond, s->condlabel ^ 1);
^
Rather than make you resend the patch, I'll just fix this up locally.
thanks
-- PMM
