https://bugs.kde.org/show_bug.cgi?id=386945
--- Comment #48 from Carl Love <c...@us.ibm.com> ---
Mark:
I took your patch " implement ldbrx as 64-bit load and Iop_Reverse8sIn64_x1" in
comment 44, attachment 116593 and applied it to current mainline.
commit f2387ed7bed64571197b44c1f8abae4d907bce1b
Author: Mark Wielaard <m...@klomp.org>
Date: Fri Nov 30 15:23:06 2018 -0500
Implement ppc64 ldbrx as 64-bit load and Iop_Reverse8sIn64_x1.
commit 206e81e8add574bbb38c11105005decedabc2f4a
Author: Mark Wielaard <m...@klomp.org>
Date: Sun Dec 2 12:39:27 2018 +0100
Fix tsan_unittest.cpp compile error with older compilers.
Older compilers (g++ 4.8.5) don't like '>>':
error: ‘>>’ should be ‘> >’ within a nested template argument list.
Add an extra space.
Then compiled and installed valgrind.
I then ran your string program in comment 45 on P7, P8 BE, P8 LE, P9 as
follows:
gcc -o mark_test ../mark_test.c
valgrind ./mark_test
On Power 7, Power 8 BE, Power 8 LE, Power 9 LE running the test:
valgrind ./mark_test
==18373== Memcheck, a memory error detector
==18373== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==18373== Using Valgrind-3.15.0.GIT and LibVEX; rerun with -h for copyright
info
==18373== Command: ./mark_test
==18373==
no
==18373==
==18373== HEAP SUMMARY:
==18373== in use at exit: 0 bytes in 0 blocks
==18373== total heap usage: 1 allocs, 1 frees, 5 bytes allocated
==18373==
==18373== All heap blocks were freed -- no leaks are possible
==18373==
==18373== For counts of detected and suppressed errors, rerun with: -v
==18373== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
Running natively an all of these systems gives:
./mark_test
no
I read thru the patch, it looks fine. I do think there might be an
easier/faster way to implement the ldbrx instruction.
Your patch does the following:
+ assign( dw1, load(Ity_I64, mkexpr(EA)) );
+ assign( dw2, unop(Iop_Reverse8sIn64_x1, mkexpr(dw1)) );
+ putIReg( rD_addr, mkexpr(dw2) );
Note, load(Ity_I64, mkexpr(EA)) does an endian aware load:
/* This generates a normal (non load-linked) load. */
static IRExpr* load ( IRType ty, IRExpr* addr )
{
if (host_endness == VexEndnessBE)
return IRExpr_Load(Iend_BE, ty, addr);
else
return IRExpr_Load(Iend_LE, ty, addr);
}
I think if you were to call IRExpr_Load to do the load as follows:
/* Load the value with the bytes reversed by doing a BE load on an LE machine
and a LE load on a BE machine. */
if (host_endness == VexEndnessBE)
assign( dw1, IRExpr_Load(Iend_LE, Ity_I64, mkexpr(EA)));
else
assign( dw1, IRExpr_Load(Iend_BE, Ity_I64, mkexpr(EA)));
putIReg( rD_addr, mkexpr(dw1) );
You wouldn't actually need to do the byte reverse with Iop_Reverse8sIn64_x1 as
the value would be loaded with the bytes reversed already. That said, having
support for the Iop_Reverse8sIn64_x1 is nice to have around for future use.
:-)
Any way, I did work thru the logic and coding for the Iop_Reverse8sIn64x1 and
it all looks correct.
Carl Love
--
You are receiving this mail because:
You are watching all bug changes.
