On Monday 02 November 2009, Aurelien Jarno wrote: > First of all I have a question about s390/s390x. It seems that you > plan to support both s390 and s390x in the same file. Is that correct?
Actually, I didn't think about supporting s390 hosts at all, but it should be possible. > Do you know how far the 32-bit support is far from compiling/working? It is not entirely unlikely that it's close. You obviously can't use the 64-bit instructions on a 31-bit host, but other than that... > Would it be really possible to support both in the same file? It would, but we would need different implementations for the 64-bit operations. From a practical point of view, I don't see any merit in supporting 31-bit hosts, simply because there aren't many in use anymore. (It's a different story for the target: 31-bit _software_ is still widespread.) > > +SEARCH_DIR("/usr/s390x-suse-linux/lib64"); > > SEARCH_DIR("/usr/local/lib64"); SEARCH_DIR("/lib64"); > > SEARCH_DIR("/usr/lib64"); SEARCH_DIR("/usr/s390x-suse-linux/lib"); > > SEARCH_DIR("/usr/lib64"); SEARCH_DIR("/usr/local/lib"); > > SEARCH_DIR("/lib"); SEARCH_DIR("/usr/lib"); > > Why adding some search directories manually here? The one you are > adding are SuSE specific. They should already be detected by the > configure script and added to config-host.ld. I merely copied the linker script and made some minor adjustments, I didn't really pay much attention to what it actually contains... > > + tcg_out32(s, 0xa7840000); /* je label1 (offset will be patched > > in later) */ > > Would be nice to have the corresponding tcg_out_ function and the > corresponding value instead of the magic value. Also I am not sure the > comment is correct here, "je" being an x86_64 instruction. objdump disassembles it as "je", though the POP doesn't specify any aliases for the different mask values for BRC. > > + tcg_out_sh64(s, SH64_SRAG, data_reg, data_reg, > > SH64_REG_NONE, 48); + break; > > + case 2 | 4: > > + tcg_out_b9(s, B9_LGFR, data_reg, arg0); > > + break; > > + case 0: case 1: case 2: case 3: default: > > + /* unsigned -> just copy */ > > + tcg_out_b9(s, B9_LGR, data_reg, arg0); > > Is it always correct? On x86_64, starting with gcc 4.3, it is not > guaranteed anymore that non used bits are set to 0. Do I understand you correctly in that you are suggesting to do an explicit zero extension here? (But what does GCC have to do with TCG-generated code?) > > + case INDEX_op_call: > > + //fprintf(stderr,"op 0x%x call 0x%lx 0x%lx > > 0x%lx\n",opc,args[0],args[1],args[2]); + if (const_args[0]) { > > + tcg_target_long off = (args[0] - > > (tcg_target_long)s->code_ptr + 4) >> 1; /* FIXME: + 4? Where did > > that come from? */ + if (off > -0x80000000 && off < > > 0x7fffffff) { /* relative call */ + tcg_out_brasl(s, > > TCG_REG_R14, off << 1); > > + tcg_abort(); // untested > > If not tested, it's probably better to remove this part and do not > mark call accepting constants. I suppose so. It's obviously never used anyway. > > > + } > > + else { /* too far for a relative call, load full > > address */ + tcg_out_movi(s, TCG_TYPE_PTR, > > TCG_REG_R13, args[0]); + tcg_out_rr(s, RR_BASR, > > TCG_REG_R14, TCG_REG_R13); > > Not sure it is very useful here. It's probably better to redefine the > constraints of the constant, so that the reg allocator use a register > to pass the value. > > > + } > > + } > > + else { /* call function in register args[0] */ > > + tcg_out_rr(s, RR_BASR, TCG_REG_R14, args[0]); > > + } > > + break; > > + case INDEX_op_jmp: > > + fprintf(stderr,"op 0x%x jmp 0x%lx 0x%lx > > 0x%lx\n",opc,args[0],args[1],args[2]); + tcg_abort(); > > + break; > > What about implementing that? Do you know a target that actually uses it? I try to avoid implementing stuff that I can't test. > > +static inline void tcg_out_addi(TCGContext *s, int reg, > > tcg_target_long val) +{ > > + tcg_abort(); > > +} > > Why this one is not implemented? This is definitely needed so that > arguments can be passed on the stack. Generally, if something is not implemented yet it's because it has never been used anywhere. > > +#define TCG_TARGET_HAS_div_i32 > > +#undef TCG_TARGET_HAS_div_i64 > > Is that corrects? It looks strange an architecture has different > division in 32- and 64-bits mode. The 64-bit instruction set actually has both. Maybe I just forgot to implement it. CU Uli -- SUSE LINUX Products GmbH, GF: Markus Rex, HRB 16746 (AG Nürnberg)