On 01/15/2018 06:27 AM, Peter Maydell wrote: > On 10 January 2018 at 05:39, Richard Henderson <r...@twiddle.net> wrote: >> The code sequence we were generating was only good for unsigned >> comparisons. For signed comparisions, use the sequence from gcc. >> >> Fixes booting of ppc64 firmware, with a patch changing the code >> sequence for ppc comparisons. >> >> Signed-off-by: Richard Henderson <r...@twiddle.net> >> --- >> tcg/arm/tcg-target.inc.c | 112 >> +++++++++++++++++++++++++++++++++-------------- >> 1 file changed, 80 insertions(+), 32 deletions(-) >> >> diff --git a/tcg/arm/tcg-target.inc.c b/tcg/arm/tcg-target.inc.c >> index 98a1253..b9890c8 100644 >> --- a/tcg/arm/tcg-target.inc.c >> +++ b/tcg/arm/tcg-target.inc.c >> @@ -239,10 +239,11 @@ static void patch_reloc(tcg_insn_unit *code_ptr, int >> type, >> } >> } >> >> -#define TCG_CT_CONST_ARM 0x100 >> -#define TCG_CT_CONST_INV 0x200 >> -#define TCG_CT_CONST_NEG 0x400 >> -#define TCG_CT_CONST_ZERO 0x800 >> +#define TCG_CT_CONST_ARM 0x0100 >> +#define TCG_CT_CONST_INV 0x0200 >> +#define TCG_CT_CONST_NEG 0x0400 >> +#define TCG_CT_CONST_INVNEG 0x0800 >> +#define TCG_CT_CONST_ZERO 0x1000 >> >> /* parse target specific constraints */ >> static const char *target_parse_constraint(TCGArgConstraint *ct, >> @@ -258,6 +259,9 @@ static const char >> *target_parse_constraint(TCGArgConstraint *ct, >> case 'N': /* The gcc constraint letter is L, already used here. */ >> ct->ct |= TCG_CT_CONST_NEG; >> break; >> + case 'M': >> + ct->ct |= TCG_CT_CONST_INVNEG; >> + break; > > In GCC constraint "M" is "integer in the range 0 to 32", which this clearly > isn't. Can we have a comment saying what it is? (may be worth mentioning > in particular how "rIM" differs from "rINK" -- I think the answer is that > we want to say "valid-immediate OR (valid-if-inverted AND valid-if-negated)", > whereas 'rINK' is "valid-immediate OR valid-if-inverted OR valid-if-negated".)
Exactly. > The code looks OK to me. I'm guessing the sparc guest failure is > just because we now generate more code for some of the comparison > cases and that doesn't fit in the buffer... I don't recall having seen a sparc failure... > We could avoid the annoying "load LE/GE immediates to tempreg" > extra code by having tcg_out_cmp2() return a flag to tell > the caller which way round to put the conditions for its two > conditional ARITH_MOV insns (for setcond2) or which condition > to use for the branch (brcond2), right? Um... we do return a condition. I must have missed a trick or made a mistake somewhere. r~
