|
From: Philippe W. <phi...@sk...> - 2015-04-10 21:24:32
Attachments:
patch_multiarch_test.txt
|
Hello, I am busy preparing a test that ensures that libVEX (somewhat) works when guest != host. I was going to soon commit the test, but now that TILEGX has been committed, the test fails miserably :(. Find attached the patch that adds the new tests and a few changes/fixes in VEX needed for this guest != host setup. When guest = amd64 and host = TILEGX, the libvexmultiarch_test asserts in TILEGX code: vex: priv/host_tilegx_defs.c:2361 (emit_TILEGXInstr): Assertion `evCheckSzB_TILEGX() == (UChar*)p - (UChar*)p0' failed. I have added some traces to see how the 'p' pointer advances. evCheckSzB_TILEGX is 80 bytes, but the 'p' pointer is only advanced by 72 bytes: ------------------------ Assembly ------------------------ EvCheck (evCheck) ld r11, 8(r50); addli r11, r11, -1; st r11, 8(r50); bgez r11, nofail; jalr *(r50); nofail: A 16 0x7fff53188880 0x7fff53188890 B 24 0x7fff53188880 0x7fff53188898 C 40 0x7fff53188880 0x7fff531888a8 D 48 0x7fff53188880 0x7fff531888b0 p1 0x7fff531888a8 E 56 0x7fff53188880 0x7fff531888b8 p1 0x7fff531888a8 F 56 0x7fff53188880 0x7fff531888b8 p1 0x7fff531888a8 p2 0x7fff531888b0 G 64 0x7fff53188880 0x7fff531888c0 H 72 0x7fff53188880 0x7fff531888c8 vex: priv/host_tilegx_defs.c:2361 (emit_TILEGXInstr): Assertion `evCheckSzB_TILEGX() == (UChar*)p - (UChar*)p0' failed. //// failure exit called by libVEX It looks like the evcheck can have a variable nr of instruction, and there is some code aiming at coping with that. But at least on guest amd64/host tilegx, that gives a problem. To reproduce, once V has been build with the patch: cd none/tests ./libvexmultiarch_test 1 0 0 Any idea what is going wrong ? Thanks Philippe NB: Having access to a TILEGX system would have allowed me to investigate the normal expected behaviour of TILEGX evcheck. But I guess at short term, we will have to debug via mail :). |
|
From: Zhigang L. <zl...@ez...> - 2015-04-10 22:19:15
|
Philippe I can look this carefully for you tonight. A quick look: the doAMode_IR( ... ) has an optimized case when the am->GXam.IR.index = 0. In this condition, the load or store will be mapped as 1 bundle, instead of 2 normally. That could be the reason. ---Zhigang -----Original Message----- From: Philippe Waroquiers [mailto:phi...@sk...] Sent: Friday, April 10, 2015 5:25 PM To: Valgrind Developers Subject: [Valgrind-developers] I need access to a TILEGX :) : libvexmultiarch_test failing with TILEGX host Hello, I am busy preparing a test that ensures that libVEX (somewhat) works when guest != host. I was going to soon commit the test, but now that TILEGX has been committed, the test fails miserably :(. Find attached the patch that adds the new tests and a few changes/fixes in VEX needed for this guest != host setup. When guest = amd64 and host = TILEGX, the libvexmultiarch_test asserts in TILEGX code: vex: priv/host_tilegx_defs.c:2361 (emit_TILEGXInstr): Assertion `evCheckSzB_TILEGX() == (UChar*)p - (UChar*)p0' failed. I have added some traces to see how the 'p' pointer advances. evCheckSzB_TILEGX is 80 bytes, but the 'p' pointer is only advanced by 72 bytes: ------------------------ Assembly ------------------------ EvCheck (evCheck) ld r11, 8(r50); addli r11, r11, -1; st r11, 8(r50); bgez r11, nofail; jalr *(r50); nofail: A 16 0x7fff53188880 0x7fff53188890 B 24 0x7fff53188880 0x7fff53188898 C 40 0x7fff53188880 0x7fff531888a8 D 48 0x7fff53188880 0x7fff531888b0 p1 0x7fff531888a8 E 56 0x7fff53188880 0x7fff531888b8 p1 0x7fff531888a8 F 56 0x7fff53188880 0x7fff531888b8 p1 0x7fff531888a8 p2 0x7fff531888b0 G 64 0x7fff53188880 0x7fff531888c0 H 72 0x7fff53188880 0x7fff531888c8 vex: priv/host_tilegx_defs.c:2361 (emit_TILEGXInstr): Assertion `evCheckSzB_TILEGX() == (UChar*)p - (UChar*)p0' failed. //// failure exit called by libVEX It looks like the evcheck can have a variable nr of instruction, and there is some code aiming at coping with that. But at least on guest amd64/host tilegx, that gives a problem. To reproduce, once V has been build with the patch: cd none/tests ./libvexmultiarch_test 1 0 0 Any idea what is going wrong ? Thanks Philippe NB: Having access to a TILEGX system would have allowed me to investigate the normal expected behaviour of TILEGX evcheck. But I guess at short term, we will have to debug via mail :). |
|
From: Philippe W. <phi...@sk...> - 2015-04-10 23:01:29
|
On Fri, 2015-04-10 at 22:19 +0000, Zhigang Liu wrote:
> Philippe
>
> I can look this carefully for you tonight.
>
> A quick look: the doAMode_IR( ... ) has an optimized case when the am->GXam.IR.index = 0.
>
> In this condition, the load or store will be mapped as 1 bundle, instead of 2 normally. That could be the reason.
That is a good hint.
When looking in main_main.c:594, we see:
switch (vta->arch_guest) {
and then depending on the guest architecture,
offB_HOST_EvC_COUNTER and offB_HOST_EvC_FAILADDR are initialised.
It sounds somewhat suspicious to initialise offB_HOST_* variables
using the guest architecture.
When we have a guest amd64, the host_EvC_FAILADDR is at offset 0
(while for TILEGX, the offset of FAILADDR is 592.
And so, the TILEGX instruction changes, due to the offset 0 being
handled specially.
We in fact use the guest offsets while we should in fact use
host offset for that.
And so, the initialisations of all offB_HOST_EvC_COUNTER and
offB_HOST_EvC_FAILADDR should in fact rather be done in
the switch at main_main.c:407
switch (vta->arch_host) {
Julian,
do you agree that the offB_HOST_* offsets are depending on the host
architecture, and not on the guest architecture ?
Philippe
|
|
From: Philippe W. <phi...@sk...> - 2015-04-10 23:11:39
|
On Sat, 2015-04-11 at 01:02 +0200, Philippe Waroquiers wrote: > Julian, > do you agree that the offB_HOST_* offsets are depending on the host > architecture, and not on the guest architecture ? Moving the offB_HOST_* to the arch_host switch makes guest amd64/host tilegx work ok. It looks to me that this is the good thing to do Note: in any case, moving these initialisations from 'guest switch' to 'host switch' can only change the values when guest != host. Philippe |
|
From: Philippe W. <phi...@sk...> - 2015-04-11 11:29:14
|
On Sat, 2015-04-11 at 01:12 +0200, Philippe Waroquiers wrote:
> On Sat, 2015-04-11 at 01:02 +0200, Philippe Waroquiers wrote:
> > Julian,
> > do you agree that the offB_HOST_* offsets are depending on the host
> > architecture, and not on the guest architecture ?
> Moving the offB_HOST_* to the arch_host switch makes
> guest amd64/host tilegx
> work ok.
>
> It looks to me that this is the good thing to do
After an irc discussion with Julian, it became clear that this
is not the good thing to do, and that I misunderstood
the somewhat misleading names offB_HOST_EvC_COUNTER and
offB_HOST_EvC_FAILADDR.
Here is what I understand now:
These offB_HOST_* are really offset in the guest state,
which give locations in the guest state that are used by the
(generated) host code.
Basically, a translation entry (generated host code) is doing
if (-- guest_state->COUNTER) == 0) goto guest_state->FAILADDR
So, COUNTER and FAILADDR are in the guest state.
FAILADDR must be an host address
(this is in fact wrongly defined in all 32 bits guest states.
E.g. libvex_guest_x86.h and libvex_guest_ppc32.h defines
UInt host_EvC_FAILADDR;
while it should be the size of an host address (or at least
big enough to hold a 64 bit host address, if the host would
be 64 bits in a multiarch setup).
So, now I think the problem guest amd64/host tilegx
is better solved in the host tilegx code, that should ensure to always
generate the same nr of bytes for the evCheck instructions
(this was suggested by Zhigang)
(or maybe dynamically compute
the needed nr of instructions for an eventcheck, depending
on the offsets of the host_EvC_*, that changes the size of the
instructions).
Zhigang, does the above look reasonable to do in tilegx ?
(waiting for this to be done, I could always disable in the test
using tilegx as a host)
Thanks
Philippe
|
|
From: Zhigang L. <zl...@ez...> - 2015-04-12 05:24:51
|
________________________________________
From: Philippe Waroquiers <phi...@sk...>
Sent: Saturday, April 11, 2015 7:30 AM
To: Zhigang Liu
Cc: Valgrind Developers
Subject: Re: [Valgrind-developers] I need access to a TILEGX :) : libvexmultiarch_test failing with TILEGX host
On Sat, 2015-04-11 at 01:12 +0200, Philippe Waroquiers wrote:
> On Sat, 2015-04-11 at 01:02 +0200, Philippe Waroquiers wrote:
> > Julian,
> > do you agree that the offB_HOST_* offsets are depending on the host
> > architecture, and not on the guest architecture ?
> Moving the offB_HOST_* to the arch_host switch makes
> guest amd64/host tilegx
> work ok.
>
> It looks to me that this is the good thing to do
After an irc discussion with Julian, it became clear that this
is not the good thing to do, and that I misunderstood
the somewhat misleading names offB_HOST_EvC_COUNTER and
offB_HOST_EvC_FAILADDR.
Here is what I understand now:
These offB_HOST_* are really offset in the guest state,
which give locations in the guest state that are used by the
(generated) host code.
Basically, a translation entry (generated host code) is doing
if (-- guest_state->COUNTER) == 0) goto guest_state->FAILADDR
So, COUNTER and FAILADDR are in the guest state.
FAILADDR must be an host address
(this is in fact wrongly defined in all 32 bits guest states.
E.g. libvex_guest_x86.h and libvex_guest_ppc32.h defines
UInt host_EvC_FAILADDR;
while it should be the size of an host address (or at least
big enough to hold a 64 bit host address, if the host would
be 64 bits in a multiarch setup).
So, now I think the problem guest amd64/host tilegx
is better solved in the host tilegx code, that should ensure to always
generate the same nr of bytes for the evCheck instructions
(this was suggested by Zhigang)
(or maybe dynamically compute
the needed nr of instructions for an eventcheck, depending
on the offsets of the host_EvC_*, that changes the size of the
instructions).
Zhigang, does the above look reasonable to do in tilegx ?
Yes, thank you for finding this issue. I have a simple patch for this, would you mind to have a try.
Thanks
--- ZhiGang
******* Begin of the patch ******
Index: priv/host_tilegx_defs.c
===================================================================
--- priv/host_tilegx_defs.c (revision 3125)
+++ priv/host_tilegx_defs.c (working copy)
@@ -1348,11 +1348,10 @@
static UChar *doAMode_IR ( UChar * p, UInt opc1, UInt rSD, TILEGXAMode * am )
{
- UInt rA; //, idx;
+ UInt rA;
vassert(am->tag == GXam_IR);
rA = iregNo(am->GXam.IR.base);
- //idx = am->GXam.IR.index;
if (opc1 == TILEGX_OPC_ST1 || opc1 == TILEGX_OPC_ST2 ||
opc1 == TILEGX_OPC_ST4 || opc1 == TILEGX_OPC_ST) {
@@ -1381,19 +1380,29 @@
return p;
}
-/* Generate a machine-word sized load or store. Simplified version of
- the GXin_Load and GXin_Store cases below. */
+/* Generate a machine-word sized load or store using exact 2 bundles.
+ Simplified version of the GXin_Load and GXin_Store cases below. */
static UChar* do_load_or_store_machine_word ( UChar* p, Bool isLoad, UInt reg,
TILEGXAMode* am )
{
+ UInt rA = iregNo(am->GXam.IR.base);
+
if (am->tag != GXam_IR)
vpanic(__func__);
- if (isLoad) /* load */
- p = doAMode_IR(p, TILEGX_OPC_LD, reg, am);
- else /* store */
- p = doAMode_IR(p, TILEGX_OPC_ST, reg, am);
-
+ if (isLoad) /* load */ {
+ /* r51 is reserved scratch registers. */
+ p = mkInsnBin(p, mkTileGxInsn(TILEGX_OPC_ADDLI, 3,
+ 51, rA, am->GXam.IR.index));
+ /* load from address in r51 to rSD. */
+ p = mkInsnBin(p, mkTileGxInsn(TILEGX_OPC_LD, 2, reg, 51));
+ } else /* store */ {
+ /* r51 is reserved scratch registers. */
+ p = mkInsnBin(p, mkTileGxInsn(TILEGX_OPC_ADDLI, 3,
+ 51, rA, am->GXam.IR.index));
+ /* store rSD to address in r51 */
+ p = mkInsnBin(p, mkTileGxInsn(TILEGX_OPC_ST, 2, 51, reg));
+ }
return p;
}
******* END of the patch ******
(waiting for this to be done, I could always disable in the test
using tilegx as a host)
Thanks
Philippe
|
|
From: Zhi-Gang L. <zh...@gm...> - 2015-04-13 22:39:41
|
Philippe
Have you ever tried the patch I post in this mail chain for the TileGX
evCheck issue?
If works for you I will commit it once I have SVN access.
Thanks
ZhiGang
On Apr 12, 2015 1:25 AM, "Zhigang Liu" <zl...@ez...> wrote:
>
>
> ________________________________________
> From: Philippe Waroquiers <phi...@sk...>
> Sent: Saturday, April 11, 2015 7:30 AM
> To: Zhigang Liu
> Cc: Valgrind Developers
> Subject: Re: [Valgrind-developers] I need access to a TILEGX :) :
> libvexmultiarch_test failing with TILEGX host
>
> On Sat, 2015-04-11 at 01:12 +0200, Philippe Waroquiers wrote:
> > On Sat, 2015-04-11 at 01:02 +0200, Philippe Waroquiers wrote:
> > > Julian,
> > > do you agree that the offB_HOST_* offsets are depending on the host
> > > architecture, and not on the guest architecture ?
> > Moving the offB_HOST_* to the arch_host switch makes
> > guest amd64/host tilegx
> > work ok.
> >
> > It looks to me that this is the good thing to do
> After an irc discussion with Julian, it became clear that this
> is not the good thing to do, and that I misunderstood
> the somewhat misleading names offB_HOST_EvC_COUNTER and
> offB_HOST_EvC_FAILADDR.
>
> Here is what I understand now:
> These offB_HOST_* are really offset in the guest state,
> which give locations in the guest state that are used by the
> (generated) host code.
> Basically, a translation entry (generated host code) is doing
> if (-- guest_state->COUNTER) == 0) goto guest_state->FAILADDR
>
> So, COUNTER and FAILADDR are in the guest state.
> FAILADDR must be an host address
> (this is in fact wrongly defined in all 32 bits guest states.
> E.g. libvex_guest_x86.h and libvex_guest_ppc32.h defines
> UInt host_EvC_FAILADDR;
> while it should be the size of an host address (or at least
> big enough to hold a 64 bit host address, if the host would
> be 64 bits in a multiarch setup).
>
>
> So, now I think the problem guest amd64/host tilegx
> is better solved in the host tilegx code, that should ensure to always
> generate the same nr of bytes for the evCheck instructions
> (this was suggested by Zhigang)
> (or maybe dynamically compute
> the needed nr of instructions for an eventcheck, depending
> on the offsets of the host_EvC_*, that changes the size of the
> instructions).
>
> Zhigang, does the above look reasonable to do in tilegx ?
>
> Yes, thank you for finding this issue. I have a simple patch for this,
> would you mind to have a try.
> Thanks
> --- ZhiGang
>
> ******* Begin of the patch ******
> Index: priv/host_tilegx_defs.c
> ===================================================================
> --- priv/host_tilegx_defs.c (revision 3125)
> +++ priv/host_tilegx_defs.c (working copy)
> @@ -1348,11 +1348,10 @@
>
> static UChar *doAMode_IR ( UChar * p, UInt opc1, UInt rSD, TILEGXAMode *
> am )
> {
> - UInt rA; //, idx;
> + UInt rA;
> vassert(am->tag == GXam_IR);
>
> rA = iregNo(am->GXam.IR.base);
> - //idx = am->GXam.IR.index;
>
> if (opc1 == TILEGX_OPC_ST1 || opc1 == TILEGX_OPC_ST2 ||
> opc1 == TILEGX_OPC_ST4 || opc1 == TILEGX_OPC_ST) {
> @@ -1381,19 +1380,29 @@
> return p;
> }
>
> -/* Generate a machine-word sized load or store. Simplified version of
> - the GXin_Load and GXin_Store cases below. */
> +/* Generate a machine-word sized load or store using exact 2 bundles.
> + Simplified version of the GXin_Load and GXin_Store cases below. */
> static UChar* do_load_or_store_machine_word ( UChar* p, Bool isLoad, UInt
> reg,
> TILEGXAMode* am )
> {
> + UInt rA = iregNo(am->GXam.IR.base);
> +
> if (am->tag != GXam_IR)
> vpanic(__func__);
>
> - if (isLoad) /* load */
> - p = doAMode_IR(p, TILEGX_OPC_LD, reg, am);
> - else /* store */
> - p = doAMode_IR(p, TILEGX_OPC_ST, reg, am);
> -
> + if (isLoad) /* load */ {
> + /* r51 is reserved scratch registers. */
> + p = mkInsnBin(p, mkTileGxInsn(TILEGX_OPC_ADDLI, 3,
> + 51, rA, am->GXam.IR.index));
> + /* load from address in r51 to rSD. */
> + p = mkInsnBin(p, mkTileGxInsn(TILEGX_OPC_LD, 2, reg, 51));
> + } else /* store */ {
> + /* r51 is reserved scratch registers. */
> + p = mkInsnBin(p, mkTileGxInsn(TILEGX_OPC_ADDLI, 3,
> + 51, rA, am->GXam.IR.index));
> + /* store rSD to address in r51 */
> + p = mkInsnBin(p, mkTileGxInsn(TILEGX_OPC_ST, 2, 51, reg));
> + }
> return p;
> }
> ******* END of the patch ******
>
>
> (waiting for this to be done, I could always disable in the test
> using tilegx as a host)
>
> Thanks
>
> Philippe
>
>
>
>
> ------------------------------------------------------------------------------
> BPM Camp - Free Virtual Workshop May 6th at 10am PDT/1PM EDT
> Develop your own process in accordance with the BPMN 2 standard
> Learn Process modeling best practices with Bonita BPM through live
> exercises
> http://www.bonitasoft.com/be-part-of-it/events/bpm-camp-virtual-
> event?utm_
> source=Sourceforge_BPM_Camp_5_6_15&utm_medium=email&utm_campaign=VA_SF
> _______________________________________________
> Valgrind-developers mailing list
> Val...@li...
> https://lists.sourceforge.net/lists/listinfo/valgrind-developers
>
|
|
From: Philippe W. <phi...@sk...> - 2015-04-14 19:36:40
|
On Mon, 2015-04-13 at 18:39 -0400, Zhi-Gang Liu wrote: > Philippe > > Have you ever tried the patch I post in this mail chain for the TileGX > evCheck issue? > > If works for you I will commit it once I have SVN access. Sorry, I did not had the chance to look at it before. I just tried on an amd64 box. With the patch, the libvexmultiarch_test can properly use tilegx as a host. (of course, I could only test that, nothing of the tilegx behaviour itself). So, whenever you commit this patch, I will cleanup the libvex_test.c code that disables tilegx as host. Thanks Philippe |