|
From: Greg P. <gp...@us...> - 2006-05-05 07:59:54
|
On ppc32, Vex's mkLoadImm() is incorrect. For some values, it
incorrectly decides that a 1- or 2-instruction load won't work,
and then fails the assertion that requires 64-bit execution for
the 5-instruction version.
My case was a Pin_Call with target=0xF01087C0. mkLoadImm() checks for
sign-extended 64-bit values, but in 32-bit mode the 32-bit target is
not sign-extended to 64-bits. (That is, ULong imm == 0x00000000F01087C0).
A quick fix is to guarantee that 32-bit mode uses the 2-instruction
version as a last resort. The check for the 1-instruction version
is still unnecessarily limiting, but I don't care about performance yet.
--- priv/host-ppc/hdefs.c (revision 1607)
+++ priv/host-ppc/hdefs.c (working copy)
@@ -2474,7 +2474,7 @@
// addi r_dst,0,imm => li r_dst,imm
p = mkFormD(p, 14, r_dst, 0, imm & 0xFFFF);
} else {
- if (imm >= 0xFFFFFFFF80000000ULL || imm < 0x80000000ULL) {
+ if (!mode64 || imm >= 0xFFFFFFFF80000000ULL || imm < 0x80000000ULL) {
// sign-extendable from 32 bits
// addis r_dst,r0,(imm>>16) => lis r_dst, (imm>>16)
(The Pin_Call being emitted was a call to ppcg_dirtyhelper_MFTB(),
which had been loaded at 0xF01087C0.)
--
Greg Parker gp...@us...
|
|
From: Julian S. <js...@ac...> - 2006-05-05 12:50:51
|
On Friday 05 May 2006 08:59, Greg Parker wrote:
> On ppc32, Vex's mkLoadImm() is incorrect. For some values, it
> incorrectly decides that a 1- or 2-instruction load won't work,
> and then fails the assertion that requires 64-bit execution for
> the 5-instruction version.
>
> My case was a Pin_Call with target=0xF01087C0. mkLoadImm() checks for
> sign-extended 64-bit values, but in 32-bit mode the 32-bit target is
> not sign-extended to 64-bits. (That is, ULong imm == 0x00000000F01087C0).
>
> A quick fix is to guarantee that 32-bit mode uses the 2-instruction
> version as a last resort. The check for the 1-instruction version
> is still unnecessarily limiting, but I don't care about performance yet.
Interesting. An alternative fix is to "manually" sign extend imm
to 64 bits in 32-bit mode. This should cause the assertion to stop
firing *and* it should make the 1-instruction version be used as
often as possible. Patch below. Can you try it?
Unfortunately I can't reproduce the problem - on SuSE 10 (ppc32)
the incoming 'imm' value appears sign-extended anyway (gcc code
generation artefact, I guess) so the patch has no effect, except
to stop the system relying on that property.
J
Index: priv/host-ppc/hdefs.c
===================================================================
--- priv/host-ppc/hdefs.c (revision 1607)
+++ priv/host-ppc/hdefs.c (working copy)
@@ -2468,6 +2468,16 @@
{
vassert(r_dst < 0x20);
+ if (!mode64) {
+ /* In 32-bit mode, make sure the top 32 bits of imm are a sign
+ extension of the bottom 32 bits, so that the range tests
+ below work correctly. */
+ UInt u32 = (UInt)imm;
+ Int s32 = (Int)u32;
+ Long s64 = (Long)s32;
+ imm = (ULong)s64;
+ }
+
if (imm >= 0xFFFFFFFFFFFF8000ULL || imm < 0x8000) {
// sign-extendable from 16 bits
|
|
From: Greg P. <gp...@us...> - 2006-05-05 18:42:44
|
Julian Seward writes: > On Friday 05 May 2006 08:59, Greg Parker wrote: > > On ppc32, Vex's mkLoadImm() is incorrect. For some values, it > > incorrectly decides that a 1- or 2-instruction load won't work, > > and then fails the assertion that requires 64-bit execution for > > the 5-instruction version. > > > > My case was a Pin_Call with target=0xF01087C0. mkLoadImm() checks for > > sign-extended 64-bit values, but in 32-bit mode the 32-bit target is > > not sign-extended to 64-bits. (That is, ULong imm == 0x00000000F01087C0). > > > > A quick fix is to guarantee that 32-bit mode uses the 2-instruction > > version as a last resort. The check for the 1-instruction version > > is still unnecessarily limiting, but I don't care about performance yet. > > Interesting. An alternative fix is to "manually" sign extend imm > to 64 bits in 32-bit mode. This should cause the assertion to stop > firing *and* it should make the 1-instruction version be used as > often as possible. Patch below. Can you try it? Yep, this works too. > Unfortunately I can't reproduce the problem - on SuSE 10 (ppc32) > the incoming 'imm' value appears sign-extended anyway (gcc code > generation artefact, I guess) so the patch has no effect, except > to stop the system relying on that property. Curious. It sure looks like doHelperCall() is intentionally truncating the target, because neither toUInt() nor Ptr_to_ULong() appear to do any sign extension. I wonder if there's an inliner bug somewhere. -- Greg Parker gp...@us... |
|
From: Julian S. <js...@ac...> - 2006-05-08 17:49:29
|
> > often as possible. Patch below. Can you try it? > > Yep, this works too. Committed. > Curious. It sure looks like doHelperCall() is intentionally truncating > the target, because neither toUInt() nor Ptr_to_ULong() appear to do > any sign extension. I wonder if there's an inliner bug somewhere. What gcc version/flags are you using ? J |